Fixes to live logs

[s0undt3ch]

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

Here's a quick checklist that should be present in PRs:

• Add a new news fragment into the changelog folder
  • name it $issue_id.$type for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of removal, feature, bugfix, vendor, doc or trivial
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
• Target: for bugfix, vendor, doc or trivial fixes, target master; for removals or features target features;
• Make sure to include reasonable tests for your change if necessary

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

• Add yourself to AUTHORS, in alphabetical order;

[nicoddemus]

Hi @s0undt3ch thanks for the PR.

Can you explain why do you think we need to capture logs during logstart and logfinish?

[coveralls]

Coverage increased (+0.006%) to 92.633% when pulling ddfff56 on s0undt3ch:master into 89a55d8 on pytest-dev:master.

[s0undt3ch]

So, I was the one who first implemented live logs, first as my own pytest plugin, which got merged to pytest catchlog plugin, and now pytest. With that said, sorry for the mess 😄

@nicoddemus the reason why those sections are being handled is because on my CI process I set 2 different log levels for live and file logs. With the latest changes, the live logs get sections which helps a ton, but the logfile just gets a ton of logs and it's really hard to know when a test started and/or finished, so I use those sections to log the start and end of tests.

Additionally on this PR, and because I was previously using pytest_runtest_protocol to log when the test started, I fixed an issue with the live logs handling when the section is unkown, which is the case for pytest_runtest_protocol or any other hook not directly handled by the live logs. This avoids a nasty traceback and a full stop when running the test suite.

And, additionally, because I think it's what was really missing from #3124, if a user explicitly set's the cli level setting, either in the ini or as an argument to pytest, automatically enable live logs. (Should we add a --live-logs CLI setting? --log-cli? ... to enable live logs from the CLI as opposed to only from the ini.

[nicoddemus]

This should be indented (under if item:) right?

[s0undt3ch]

Not necessarily because item does exist on teardown. But it can be moved.

[nicoddemus]

Oh you are right, well up to you!

[nicoddemus]

I'm not sure about this, I can imagine someone setting log_cli_level in their pytest.ini file so that they can enable/disable live logging in the command line without having to also pass the log level each time...

@Thisch what do you think?

[s0undt3ch]

In that case I think we need a CLI flag to enable live logs.

[nicoddemus]

We have the log_cli configuration option, which can be changed directly in the command-line: -o log_cli=True. 😉

[twmr]

I'm not sure if setting log_cli_level on the command line or in an ini file should automatically set log_cli to True. I would prefer if we have a similar shortcut for live logging, like we have for disabling stream capturing (-s).

[s0undt3ch]

@Thisch --log-cli? --logs-stream?

[nicoddemus]

I'm OK about having a --log-cli flag, although it is so close to -o log_cli=true that I'm not sure it adds much. What about having a short option for it instead? And what that would be?

[twmr]

Maybe -S in analogy to -s?

[nicoddemus]

Too close IMHO... @RonnyPfannschmidt are you 👎 on having a command-line for this, as you mentioned in #3190 (comment)?

[nicoddemus]

Thanks @s0undt3ch and sorry for the delay!

So, I was the one who first implemented live logs, first as my own pytest plugin, which got merged to pytest catchlog plugin, and now pytest. With that said, sorry for the mess

Not at all, thanks for all your work! 😁

Additionally on this PR, and because I was previously using pytest_runtest_protocol to log when the test started, I fixed an issue with the live logs handling when the section is unkown, which is the case for pytest_runtest_protocol or any other hook not directly handled by the live logs. This avoids a nasty traceback and a full stop when running the test suite.

This is great, but I believe we should split this PR:

1. Fix the bug above (also reported in Live logger error before 'when' is set #3184), targeting master.
2. Feature about catching logs during pytest_runtest_logstart and pytest_runtest_logfinish, targeting features.
3. Feature about automatically enable live logging if cli_log_level is given (although this I'm not so sure if this is a good idea), targeting features.

[nicoddemus]

Please see my comments! 👍

[s0undt3ch]

Ok. I'll split these up tomorrow morning. Thanks for the review.

[nicoddemus]

Ok. I'll split these up tomorrow morning. Thanks for the review.

Thanks! Looking forward to the PRs. 👍

[s0undt3ch]

So, #3188 and #3189 address 1 and 2 respectively.

As for 3, let me know your preference and I'll submit another PR.

This can probably be closed now.

[nicoddemus]

Thanks @s0undt3ch!