Deprecate --no-print-logs option

[nicoddemus]

As commented in #3234 (comment) we want to deprecate --no-print-logs.

---

@thisch to deprecate we basically should emit config.warn when the option is set, at first glance here seems like a good place:

pytest/_pytest/logging.py

Line 346 in b486e12

self.print_logs = get_option_ini(config, 'log_print')

We should call config.warn if self.print_logs (after accounting for command-line and ini option) is False.

Then we should add a test to deprecated_test.py and I believe that's it. As usual feel free to ask if you have questions. 😁

[nicoddemus]

Oh also update the docs deprecating the feature. 😁

[vinaycalastry]

I can pick this up if no one else is currently working on this.

[vinaycalastry]

I think I'm done with this change. Only need to know what do you mean by " (after accounting for command-line and ini option)"

[nicoddemus]

It means we should make sure the option was actually given either in the command-line or as ini-option (instead of the default being used because the user did not specify it using either method).

I think you can accomplish this by changing the default to None:

    add_option_ini(
        "--no-print-logs",
        dest="log_print",
        action="store_const",
        const=False,
        default=None,
        type="bool",
        help="disable printing caught logs on failed tests.",
    )

Because now when we obtain the option value and it is not None, we know the user did specify it explicitly, then we can issue our warning and fallback to the actual default:

self.print_logs = get_option_ini(config, "log_print")
if self.print_logs is not None:
    # user did specify the option explicitly, warn about deprecation while
    # mentioning that they should use "--show-capture"
    warn(...)
else:
    showcapture = config.getoption("showcapture")
    self.print_logs = showcapture in ("all", "log")

@thisch please correct me if I'm wrong.

[vinaycalastry]

makes sense. Thank you!
Also, looks like the warnings are now handled differently, I'm using the warnings module now.
This was supposed to be removed in V4.0, but since pytest is already at 5.x, does it make sense to remove it completely.

Or else, we could add it to deprecations first and then remove it at a later time.

I can add it to deprecated.py (https://github.com/pytest-dev/pytest/blob/master/src/_pytest/deprecated.py) and deprecate this like resultlog was done.
Also, can you tell me what stacklevel means in warnings.warn() method.

Sorry for all these questions :)

[nicoddemus]

This was supposed to be removed in V4.0, but since pytest is already at 5.x, does it make sense to remove it completely.

Or else, we could add it to deprecations first and then remove it at a later time.

Because we never did deprecate it, we should deprecate it now and remove in the next major release.

I can add it to deprecated.py (/src/_pytest/deprecated.py@master ) and deprecate this like resultlog was done.

Sounds good, thanks!

Also, can you tell me what stacklevel means in warnings.warn() method.

See https://docs.python.org/3/library/warnings.html#warnings.warn, but in this case it is meaningless because the warning is not triggered in user-code land, so leave it alone in this case.

Also, try it yourself on the command-line, if you don't see a warning, you might need to use _issue_warning_captured instead (like resultlog.py does it).

[vinaycalastry]

I added unit tests to check if both ini or command-line options are set and display the Deprecation warning.

I used _issue_warning_captured instead of using warnings.warn() method directly as I couldn't get the unit tests to work even though I could display the warning by running the pytest command.

#6328

[vinaycalastry]

Hey, So I made a mistake trying to rebase and since there were a lot of commits, I closed the old PR and made a new one
#6333

[nicoddemus]

Fixed by #6333 in features.