docs: remove log_cli and log_file ini options; keep them as cli arguments for integration tests#2654
Conversation
…ents for integration tests
| ``` | ||
|
|
||
| You can also configure pytest logging options on the command line. | ||
| You can also configure these options on the command line. If you don't see the live Jubilant logs, make sure the pytest command (for example, in `tox.ini`) has the `--log-cli-level=INFO` argument. |
There was a problem hiding this comment.
I changed pytest logging to these to keep it consistent with the same sentence in Log to a file section.
There was a problem hiding this comment.
I think "pytest logging options" works better in this case because there's a sample output directly above, and it might be a little unclear what "these" refers to.
For the second sentence, would you mind explaining this to me again? If log_cli_level = "DEBUG" is already set in pyproject.toml, why wouldn't I see the live logs? And why are we suggesting to set it to INFO on the CLI instead of DEBUG?
There was a problem hiding this comment.
I think "pytest logging options" works better in this case because there's a sample output directly above, and it might be a little unclear what "these" refers to.
Oh, sorry. Somehow I thought this was a typo made by me in the last PR. I will revert it. I agree that pytest logging works better.
log-cli-level = "DEBUG" is already set in pyproject.toml, why wouldn't I see the live logs?
If you set log_cli_level = <level> and not set log_cli = true in pyproject.toml, live logging is disabled. To enable live logging, without using log_cli = true, we need an equivalent CLI argument. However, there is no --log-cli argument in pytest 😞 (here). The only way we could enable live logging from CLI is to use --log-cli-level=<level>, which implicitly enable live logging.
And why are we suggesting to set it to INFO on the CLI instead of DEBUG?
Ah thanks for pointing this out. In this section, we have 1 example for brief terminal logging and 1 for verbose terminal logging. Because we need --log-cli-level=<level> to enable live logging from CLI, level should be the same level defined by log_cli_level in pyproject.toml.
The sentence I have here is confusing. How about:
If you don't see the verbose Jubilant logs,
make sure the pytest command (for example, in `tox.ini`)
has the `--log-cli-level=DEBUG` argument.
We don't need to care about brief logs here because the default tox.ini already set that up.
[testenv:integration]
...
commands =
pytest \
-v \
-s \
--tb native \
--log-cli-level=INFO \
{[vars]tests_path}/integration \
{posargs}
As an additional improvement, we can even remove log_cli_level from pyproject.toml completely, and rely on its CLI version --log-cli-level to prevent confusion.
There was a problem hiding this comment.
OK, I think I understand, thanks!
So this is how I understand it:
pyproject.toml:
log_cli_format = "%(levelname)s %(name)s %(message)s"
tox.ini:
[testenv:unit]- No logging options on thepytestcommand. No live logging. In effect,log_cli_formatinpyproject.tomlis ignored.[testenv:integration]---log-cli-level=INFOon thepytestcommand. Produces brief logs.
Then for verbose logs during integration tests, we recommend the developer changes two places: Switch to --log-cli-level=DEBUG in tox.ini, and adjust pyproject.toml to have:
log_cli_format = "%(asctime)s %(levelname)s %(name)s %(message)s"
log_cli_date_format = "%Y-%m-%dT%H:%M:%SZ"
Did I get that right?
There was a problem hiding this comment.
Yes that is correct!
There was a problem hiding this comment.
Great 🙂 I would suggest a couple of changes to the doc in that case. Instead of having an "If you don't see..." sentence here, I'd tell people how we expect them to use the pytest command. Something like this (which includes a few other adjustments):
Example of brief logging
In
pyproject.toml:[tool.pytest.ini_options] ... log_cli_format = "%(levelname)s %(name)s %(message)s"Also check the
[testenv:integration]environment intox.inito make sure the pytest command has--log-cli-level=INFO.Sample output of
tox -e integration:[...]
Example of verbose logging
In
pyproject.toml:[tool.pytest.ini_options] ... log_cli_format = "%(asctime)s %(levelname)s %(name)s %(message)s" log_cli_date_format = "%Y-%m-%dT%H:%M:%SZ"The timestamp format follows ISO 8601.
See more: {external+python:ref}
strftime() and strptime() behavior <strftime-strptime-behavior>Also modify the
[testenv:integration]environment intox.iniso that the pytest command has--log-cli-level=DEBUG.Sample output of
tox -e integration:[...]
There was a problem hiding this comment.
This is great. Thank you so much.
I updated the doc to use this version.
| Use the `--log-file` option from pytest to enable file logging: | ||
|
|
||
| ```text | ||
| tox -e integration -- --log-file logs/verbose.log |
There was a problem hiding this comment.
From our daily, my impression is that we are okay with leaving --log-file logs/verbose.log here.
When we update the Charmcraft profiles with GitHub Action configuration, --log-file logs/verbose.log can go into tox.ini or Makefile. We want the behaviour to be consistent running the integration tests command both in CI and local.
There was a problem hiding this comment.
What would be the downside of adding --log-file logs/verbose.log to tox.ini now rather than later?
There was a problem hiding this comment.
Hmmm, there is no practical downside to usability as far as I know of. I think the decision was more about: implementing log to file together with the GitHub Action workflow config and maintaining consistency.
tonyandrewmeyer
left a comment
There was a problem hiding this comment.
Looks good to me, thanks! I like David's suggestion for the configuration, no other comments from me.
| @@ -569,16 +573,19 @@ This is an ideal configuration for long-running integration tests (for example, | |||
| # Otherwise, that section will have DEBUG logs (coming from log_file_level). | |||
| log_level = "INFO" | |||
There was a problem hiding this comment.
I just had one thought. Does log_level = "INFO" mean it doesn't matter what --log-cli-level is set to in tox.ini? If tox.ini has --log-cli-level=DEBUG, will pytest output brief or verbose logs to the console? Will log_level = "INFO" take precedence?
If log_level = "INFO" doesn't take precedence, I think we need to include the same sentence as earlier:
Also check the [testenv:integration] environment in tox.ini to make sure the pytest command has --log-cli-level=INFO
We could put this immediately after the pyproject.toml sample.
There was a problem hiding this comment.
Oh good point. Thank you. --log-cli-level will take precedence.
If tox.ini has --log-cli-level=DEBUG, pytest will output verbose logs to the console.
I will add the sentence in.
In #2619, I added the
Configure Jubilant logshowto section.In this guide, I set
log_cliandlog_filein the[tool.pytest.ini_options]table inpyproject.toml.log_cli = trueenables live logging for all pytest invocation.log_file = <path>enables file logging for all pytest invocation.This works well for
tox -e integration. However, as a side effect,tox -e unitemitted live logs from the charm's code. This was mentioned by @dwilding.We don't want this behaviour in unit tests.
This PR fixes the howto guide:
log_cli = true,log_cli_level = <level>andlog_file = <path>from all suggested configs.tox -e integrationprovides--log-cli-level=INFOto the pytest command, which enable live logging.tox -e integration -- --log-file logs/verbose.logwill emit the verbose logs tologs/verbose.log. This requires changes to the GitHub Action workflow example.juju debug-logfrompytest-jubilantthe command will betox -e integration -- --log-file logs/verbose.log --juju-dump-logs logs. See a test run herePreview