integrates pytest-metadata and extract verbose to own argument #19
Conversation
|
Thank you for submitting a pull-request to pytest-md, @jupe! 📝 |
use list style to make md more readable
| """ Create environment section for the Markdown report.""" | ||
| outcome_text = "" | ||
|
|
||
| def _generate_md(items, indentation=0): |
There was a problem hiding this comment.
Please try to avoid recursion for the sake of comprehensibility. This is how pytest-html generates the metadata section https://github.com/pytest-dev/pytest-html/blob/master/pytest_html/plugin.py#L558
| @@ -236,6 +263,18 @@ def pytest_addoption(parser): | |||
| default=None, | |||
| help="create markdown report file at given path.", | |||
| ) | |||
| group.addoption( | |||
There was a problem hiding this comment.
I'd prefer to add metadata to the Markdown report based on whether pytest-metadata is enabled rather than adding an additional CLI option.
There was a problem hiding this comment.
It might lead unexpected "backwards compatible break" behaviour. User might want to use pytest-metadata and pytest-md and expects that generated markdown is still the same even new version is deployed. This is a bit similar thing that verbose usage earlier (#18), right ? PR makes possible to export metadata to markdown or not regardless pytest-metadata plugin installation. Maybe it's okay even it's always "on". In my case I want to generate markdown document and post it to PR comment. Currently markdown document is a bit annoying long - that's why I implemented also collapse metadata/individual test results here.
| dest="md_metadata", | ||
| help="Add environment section to report", | ||
| ) | ||
| group.addoption( |
There was a problem hiding this comment.
I'd honestly prefer if we stick to the current approach and use --verbose instead of adding an extra CLI option and breaking backwards compatibility. 🤔
There was a problem hiding this comment.
a bit inconsistent propose against to metadata propose that would make breaks ;) But currently I can survive with original approach without extra CLI option since I did collapsed verbose section in my fork ... :) What's final decision? :) reject original issue #18 or accept it... ?
| # Check the generated Markdown report | ||
| assert report_path.read_text() == report_content | ||
| if isinstance(report_content, Pattern): |
There was a problem hiding this comment.
Let's create a new test for when pytest-metadata is enabled.
There was a problem hiding this comment.
there is already test with pytest-metadata is installed and enabled. Did you mean disabled but still installed ?
There was a problem hiding this comment.
added test with emoji and metadata enabled. Did you mean that ?
|
@hackebrot could you check my comments and give some feedback how to continue with PR. Would like to finish this work and drop my fork :) |
|
@hackebrot could you add comments here since would like to finish my PR... ? :) |
new features
Generate Test Metadata section to markdown report using
Covers: #8
Breaking change
Extract md
--verboseto own md specific argument. This makes possible to run tests with verbose mode but control reporter separately.Does not affect generated markdown.
Fixes #18
PR also added one example that simplify testing plugin functionality.