Add hook to change report main title #270
Conversation
|
Thanks for this PR @werdeil ! Couple of comments:
Feel free to reach out if you need any guidance. Thanks again! |
pytest_html/plugin.py
Outdated
| @@ -490,9 +490,12 @@ def generate_summary_item(self): | |||
| __name__, os.path.join("resources", "main.js") | |||
| ).decode("utf-8") | |||
|
|
|||
| report_title = os.path.basename(self.logfile) | |||
| session.config.hook.pytest_html_results_report_title(report_title=report_title) | |||
There was a problem hiding this comment.
Isn't the behavior for this a bit confusing? I thought strings were immutable.
>>> def swap(rt):
... rt = 'hi'
...
>>> rt = 'report title'
>>> swap(rt)
>>> rt
'report title'
>>>
How can the hook modify the report_title in place? Shouldn't the hook return a value instead?
There was a problem hiding this comment.
Indeed, did think it about enough, sorry.
I see two solutions:
- the hook return a value but it doesn't work as the other hooks.
- the hook takes a mutable as input and we keep this behavior
I think the second option is better, and I'll push a commit to give the whole html title to the hook. It will allow more title customization.
Just need to document it, add test and examples before submitting again.
|
I think I've covered all review comments, Travis tests are failing but I think this is not related to my changes, I would like to rerun it but I don't have the right to do it. @BeyondEvil @csm10495 Tell me if I need to do anything more. |
|
Is there a specific use-case for the hook @werdeil ? With the way it is implemented, it's a lot of code (perhaps even the introduction of a conftest file) just to set the report title. If there's not a specific use-case that warrants a hook, maybe a command line argument and/or an ini-file key will suffice? See this comment |
|
My use case is to have a CI pipeline that generates a customized report.html. The CI generates automatically the title, and may add dynamically version or number of pass/fail e.g. "TEST REPORT v1.2.3 [2/3 Passed]" In my use case I already have a conftest, and use other hooks so having another hook seems logic. Having a command-line and/or an ini file key will indeed be sufficient except in cases where we want to add some test results in it. |
|
Ok, fair enough! Thanks for the detailed explanation. So, I guess with that, what I would like to see is to make it a little more straight forward to set the title than to have to mess around with html etc. Here's my suggestion: Add a Then you can make the hook like so: def pytest_html_report_title(report):
""" Called before adding the title to the report """And the plugin can call the hook: session.config.hook.pytest_html_report_title(report=self)And an implementation can then be made like: def pytest_html_report_title(report):
report.title = "My very own title!"Also, note the name change of the hook. The title is for the entire report, not only the results part. 😊 Hope that makes sense. Feel free to question it. :) |
|
Ok I'll follow your suggestion but I have one remark though: as we give the report to the hook, it allows to change all parts of the report, not only the title. I see that we have already hooks that take the report as input so I guess it is acceptable. My concern is that someone willing to change anything in the report will use the "pytest_html_report_title" hook instead of finding a "better" way to do it, but as I said, it is already possible though other hooks. |
|
Released in v2.1.0 |
I think it is a good thing to be able to have a customizable report title that is not always the same as the file name.
As it is my first merge request on this project I'm open to any comment.