Don't hardcode 'std' for captured stdout/stderr. #1052
Conversation
This will make Item.add_report_sect more usable for plugins. See eisensheng/pytest-catchlog#7
|
Is there a chance you could add a test for Also if we start to consider this method as an external API it should grow a docstring so that doc/en/writing_plugins.rst "Item" docs pick it up. |
|
I haven't actually taken a closer look at eisensheng/pytest-catchlog#7 yet - I wonder what's the right way to proceed. Should @nicoddemus what's your take on this? It seems /cc @abusalimov |
|
FWIW on @pytest.mark.hookwrapper
def pytest_runtest_makereport(self, item, call):
outcome = yield
report = outcome.result
...
if not report.passed:
long_repr = getattr(report, 'longrepr', None)
if hasattr(long_repr, 'addsection'):
...
if lines:
long_repr.addsection('Captured Qt messages', '\n'.join(lines))(full code here) I didn't know about I could replace all the above by simply calling I think we should make it part of the public API, along with adding a Nevertheless, this PR should be merged as solves a small wart. 👍 |
|
i agree it should be merged but at least there should be docs (see my previous comment). Also, shouldn't the PR target features instead of master? |
I hope I can add docs today or tomorrow. As for tests, the functionality is already tested by
I'm not sure. I didn't think of this PR as making If you consider this as a new added feature, maybe that'd be better? |
I agree. if it was becoming public then it would be different, but here an existing feature is being documented better. Even if someone makes use of it by looking at the docs for |
|
we should note that the api of that method is completely wrong the assumption is that the added section has something to do with capture we should ensure to deprecate that one quickly and think about a better way to add sections to reports |
|
the assumption is that the added section has something to do with capture
it ignores any semantics of when
Sorry, may be I couldn't parse you message properly, but the method in
question does have a 'key' argument, which is used as 'when'.
|
|
@abusalimov however the sections are accumulated over all when actions, so a call report also adds the items of a setup, and a teardown report also adds the sections of call and setup when in that part of the code is purely cosmetic, its not considered for semantics |
|
@RonnyPfannschmidt I guess, you're right. Probably a better place for storing sections would be an instance of BTW, what is the rationale for |
|
@abusalimov a call is directly evaluated and then has the result information of that call, thus making it a parameter to the other hooks is semantically wrong if it was not directly evaluated, we would have a huge semantic missmatch |
|
@RonnyPfannschmidt Is it that wrong to pass |
Then we should hold on adding docs for it as it would elevate it even more to "public" status. Can we merge this PR then? The "wart removal" is worth it, and it seems |
|
@abusalimov it would completely change the scope of callinfo - i'm strictly opposed unless someone presents a good reason |
|
@The-Compiler please add to the doc-string that this function is accidentally under a public name and scheduled for removal/replacement, once we have the replacement we can add in a pytest-warning and schedule it for complete removal in 3.0 |
Don't hardcode 'std' for captured stdout/stderr.
Take advantage of `@pytest.mark.hookwrapper` facility and use more
idiomatic pytest_runtest_call(item) hook for performing capturing (in
fact, the same is used by the core 'pytest.capture' module).
Instead of messing around `report.longrepr` internals and since we
don't have a `report` instance at the moment of executing
pytest_runtest_call() anyway (it used to be created within a more late
pytest_runtest_makereport() hook), use a higher-level
item.add_report_section() method to attach log output to a report.
However, there is a drawback that now reports show sections named like
'Captured stdlog ...' ('std' is hardcoded). This is an issue of the
pytest itself addressed in pytest-dev/pytest#1052. That PR also
contains a discussion on deprecating Item.add_report_section(),
however the same approach is still used by 'pytest.capture', so it
should be fine to use it here as well.
This change also removes the use of the deprecated '__multicall__', so
that fixes eisensheng#3.
Take advantage of `@pytest.mark.hookwrapper` facility and use more
idiomatic pytest_runtest_call(item) hook for performing capturing (in
fact, the same is used by the core 'pytest.capture' module).
Instead of messing around `report.longrepr` internals and since we
don't have a `report` instance at the moment of executing
pytest_runtest_call() anyway (it used to be created within a more late
pytest_runtest_makereport() hook), use a higher-level
item.add_report_section() method to attach log output to a report.
However, there is a drawback that now reports show sections named like
'Captured stdlog ...' ('std' is hardcoded). This is an issue of the
pytest itself addressed in pytest-dev/pytest#1052. That PR also
contains a discussion on deprecating Item.add_report_section(),
however the same approach is still used by 'pytest.capture', so it
should be fine to use it here as well.
This change also removes the use of the deprecated '__multicall__', so
that fixes eisensheng#3.
Take advantage of `@pytest.mark.hookwrapper` facility and use more
idiomatic pytest_runtest_call(item) hook for performing capturing (in
fact, the same is used by the core 'pytest.capture' module).
Instead of messing around `report.longrepr` internals and since we
don't have a `report` instance at the moment of executing
pytest_runtest_call() anyway (it used to be created within
pytest_runtest_makereport() hook), use a higher-level
item.add_report_section() method to attach log output to a report.
However, there is a drawback that now reports show sections named like
'Captured stdlog ...' ('std' is hardcoded). Nevertheless this is likely
to be an issue of the pytest itself. It is addressed in
pytest-dev/pytest#1052, and that PR also contains a discussion on
deprecating Item.add_report_section(). However, the same approach is
still used by 'pytest.capture', so it should be fine to use it here as
well.
This change also removes the use of the deprecated '__multicall__', so
that fixes eisensheng#3.
This will make
Item.add_report_sectmore usable for plugins.See eisensheng/pytest-catchlog#7