Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation for caplog set_level() should make it clearer it's a threshold #9146

Closed
MetRonnie opened this issue Oct 1, 2021 · 17 comments · Fixed by #9149
Closed

Documentation for caplog set_level() should make it clearer it's a threshold #9146

MetRonnie opened this issue Oct 1, 2021 · 17 comments · Fixed by #9149
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification

Comments

@MetRonnie
Copy link
Contributor

MetRonnie commented Oct 1, 2021

What's the problem this feature will solve?

The documentation for caplog.set_level() says

Set the level of a logger for the duration of a test.

This does not make it clear that it's setting the threshold level for capturing logging.

Describe the solution you'd like

The Python logging documentation makes this clear for logging.setLevel():

Sets the threshold for this logger to level. Logging messages which are less severe than level will be ignored; ...

caplog.set_level() ought to do the same.

Additional context

The only reason I know it's a threshold is because I tried running pytest -rA with different levels set in caplog and the output contained messages at that level and above

@nicoddemus
Copy link
Member

Thanks @MetRonnie for the detailed report.

Would you like to contribute that change?

@nicoddemus nicoddemus added good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification labels Oct 1, 2021
@MetRonnie
Copy link
Contributor Author

MetRonnie commented Oct 1, 2021

Happy to, will open a PR from my non-work account

@Vijay-Arora
Copy link
Contributor

Hey @nicoddemus and @MetRonnie, I am new to contributing to open source and I'm looking for a good first issue. Could you please kindly guide me if I can pick up this or any issue which you think might be useful for me to start with?
Thanks in advance!

@nicoddemus
Copy link
Member

Thanks @MetRonnie and @Vijay-Arora.

Just need to update the docstring at:

def set_level(self, level: Union[int, str], logger: Optional[str] = None) -> None:
"""Set the level of a logger for the duration of a test.
.. versionchanged:: 3.4
The levels of the loggers changed by this function will be
restored to their initial values at the end of the test.
:param int level: The level.
:param str logger: The logger to update. If not given, the root logger.
"""

Vijay-Arora added a commit to Vijay-Arora/pytest that referenced this issue Oct 1, 2021
Updated logging.py for pytest-dev#9146
@Vijay-Arora
Copy link
Contributor

Hey @nicoddemus and @MetRonnie, I did it! As it's my first time, let me know in case any change is required.

@Vijay-Arora
Copy link
Contributor

Also @nicoddemus, I first forked it and then contributed; I was wondering if we can directly open a PR without forking and creating our own branch in this repository itself? Is that allowed?

@nicoddemus
Copy link
Member

I was wondering if we can directly open a PR without forking and creating our own branch in this repository itself? Is that allowed?

Only for project contributors (as there are security concerns to let anyone commit to the repository).

@nicoddemus
Copy link
Member

Also I don't see a PR? You might have just pushed that branch to a fork, but you need to open a Pull Request. Consult GitHub's help for more information. 👍

@Vijay-Arora
Copy link
Contributor

Oh okay. Thanks for the quick response, @nicoddemus!

@Vijay-Arora
Copy link
Contributor

Also I don't see a PR? You might have just pushed that branch to a fork, but you need to open a Pull Request. Consult GitHub's help for more information. 👍

Hmm, okay, thanks for the heads up! Will update here as soon as I see how to make that work.

Vijay-Arora added a commit to Vijay-Arora/pytest that referenced this issue Oct 1, 2021
@Vijay-Arora
Copy link
Contributor

Vijay-Arora commented Oct 1, 2021

Hey @nicoddemus, I added a PR; it's visible now but i think i missed some of the points. Could you please take a look at it and give me some feedback on it?
Thanks!

Vijay-Arora added a commit to Vijay-Arora/pytest that referenced this issue Oct 1, 2021
@kushagrabhushan
Copy link

Hello @Vijay-Arora @nicoddemus @MetRonnie! I am also just getting started with open-source and wanted to start contributing. I just had a question whether this issue is currently open or has it been closed?

@MetRonnie
Copy link
Contributor Author

@kushagrabhushan This is still open but there is already a pull request #9149 to close it (you can see this in the "Linked pull requests" section in the sidebar at the top)

@Vijay-Arora
Copy link
Contributor

@kushagrabhushan, I'm actively working on it, I'll be picking this up from where I left this on Friday.

@DaveParr
Copy link

Is this still needing action and able to be acted on?

I'm looking to get invoolved in hacktoberfest 2022, and this issue is now a year old. FWIW I understand from the users persepective how pytest works and how the caplog process can be utilised in tests.

@tuhinmallick
Copy link

Is this issue still open, if yes i would like to work on it

@nicoddemus
Copy link
Member

@Vijay-Arora was working on it it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue easy issue that is friendly to new contributor type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants