Add captured-log support to --show-capture

[twmr]

Fixes: #3233

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs (you can delete this text from the final description, this is
just a guideline):

• Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.
• Target the master branch for bug fixes, documentation updates and trivial changes.
• Target the features branch for new features and removals/deprecations.
• Include documentation when adding new features.
• Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

• Add yourself to AUTHORS in alphabetical order;

[coveralls]

Coverage increased (+0.05%) to 92.623% when pulling acda6c4 on thisch:showcapture_log_support into 3bc8b50 on pytest-dev:features.

[nicoddemus]

Awesome @thisch, thanks! Please just update the other changelog entry as I requested, after this we can merge this.

Scratch that, as soon as CI passes we can merge this and then update the other changelog entry separately (even from GH's interface directly).

[nicoddemus]

Good thinking, just realized that we definitely should have used all instead of both from the beginning. 😉

[nicoddemus]

This reminds me, please update 1478.feature as well. 👍

[twmr]

Does towncrier reference this PR (#3234) then, or do we have to update the generated changelog manually?

[nicoddemus]

It uses the number in the filename to create a link, so the file above will link to #3204

[twmr]

I would like to deprecate --no-print-logs, which is no longer needed when we merge this PR. I guess that I can simply remove this option and we mention it in the changelog somewhere, right?

[nicoddemus]

I would like to deprecate --no-print-logs, which is no longer needed when we merge this PR. I guess that I can simply remove this option and we mention it in the changelog somewhere, right?

Unfortunately this is an API breakage, so we can only actually remove it in 4.0, we should deprecate it for now and create an issue to remove it in 4.0 using the deprecation label.

[nicoddemus]

Also I was thinking: the --show-capture option needs to accept multiple values, so users which don't want logging can pass --show-capture=stdout --show-capture=stderr to disable log capture output (which seems like a long command line now that I see it).

Perhaps accepting multiple values is best: --show-capture=stdout,stderr.

[nicoddemus]

Please deprecate --no-print-logs

[twmr]

Ok I'll revert my latest commit, but does it make sense to keep the changes made to the logging docu?

--- a/doc/en/logging.rst
+++ b/doc/en/logging.rst
@@ -50,26 +50,10 @@ These options can also be customized through ``pytest.ini`` file:
     log_format = %(asctime)s %(levelname)s %(message)s
     log_date_format = %Y-%m-%d %H:%M:%S

-Further it is possible to disable reporting logs on failed tests completely
-with::
+Further it is possible to disable reporting of captured content (stdout,
+stderr and logs) on failed tests completely with::
 
-    pytest --no-print-logs
-
-Or in the ``pytest.ini`` file:
-
-.. code-block:: ini
-
-  [pytest]
-  log_print = False
-
-
-Shows failed tests in the normal manner as no logs were captured::
-
-    ----------------------- Captured stdout call ----------------------
-    text going to stdout
-    ----------------------- Captured stderr call ----------------------
-    text going to stderr
-    ==================== 2 failed in 0.02 seconds =====================
+    pytest --show-capture=no

[nicoddemus]

@thisch it does, thanks! Do you want to deprecate --no-print-logs now or in a separate task? And what do you think about the idea of --showcapture receiving multiple parameters?

[twmr]

I've partially reverted my "remove --no-print-logs" commit now.

Honestly, I don't see the use-case for multiple parameters for --show-capture. I think that nobody wants to disable e.g. only the log-report output but keep the stdout/stderr-report output of failed tests.

Maybe @s0undt3ch and @abusalimov want to comment on this, since they implemented/reviewed eisensheng/pytest-catchlog#34. (The question is the following: "are the 5 options for --show-capture (no, stdout, stderr, log, all) enough or do we want multiple parameters for --show-capture as suggested by @nicoddemus in #3234 (comment)?)

[nicoddemus]

I've partially reverted my "remove --no-print-logs" commit now.

Thanks!

Honestly, I don't see the use-case for multiple parameters for --show-capture. I think that nobody wants to disable e.g. only the log-report output but keep the stdout/stderr-report output of failed tests.

I'm not sure, I suppose there are people that don't want to see logging output at all, but want to keep stdout/stderr intact (which is pytest's <3.3) behavior (see #3046). If nothing else, it might be problematic to simply remove/deprecate an option without offering an equivalent alternative.

[twmr]

I'm not sure, I suppose there are people that don't want to see logging output at all, but want to keep stdout/stderr intact (which is pytest's <3.3) behavior (see #3046). If nothing else, it might be problematic to simply remove/deprecate an option without offering an equivalent alternative.

If they want that for whatever reason they can set the log-level to e.g. CRITICAL (-o log_level=CRITICAL) or disable the logging plugin using -p no:logging. I think that adding a --no-pirint-logs option was easier for the catchlog devs than completely disabling the output of all sections at the end of failed tests using a flag, which they probably wanted.

[nicoddemus]

If they want that for whatever reason they can set the log-level to e.g. CRITICAL (-o log_level=CRITICAL) or disable the logging plugin using -p no:logging. I think that adding a --no-pirint-logs option was easier for the catchlog devs than completely disabling the output of all sections at the end of failed tests using a flag, which they probably wanted.

Indeed you got a point, and even better that solution can be controlled at the testing level (I mean setting the loglevel in a test using caplog). Well, to make --show-capture support multiple values was just an idea that we can leave to implement later if needed, specially because it is backward compatible. If somebody uses --no-print-logs because they want to disable logging while keeping stdout/stderr intact, they will see a warning and will probably raise the question of what to use instead, in which case we can direct them to the docs and see what feedback we get. 👍

[twmr]

@nicoddemus how should I deprecate the --no-print-logs option? (I've never deprecated a pytest feature)

[nicoddemus]

Let's do it in #3238, we can merge it now. 👍 Thanks for all the work and patience!