Skip to content

better handle errors raised while applying filterwarnings (#7864) #7877

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

Conversation

charlesaracil-ulti
Copy link
Contributor

closes #7864

Add a more explicit error when raising exception while trying to import filterwarnings.

@charlesaracil-ulti charlesaracil-ulti force-pushed the handle_errors_warning_filters branch from df230aa to c1eb283 Compare October 9, 2020 14:01
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @charlesaracil-ulti.

Some comments:

  1. It would be good if the error says which warning filter causes the problem.

  2. The function parse_warning_filter can fail in other places than just the warnings._getcategory call. It would be good to handle all sort of invalid warning filters while we're at it.

  3. Right now the exception would be raised "lazily", i.e. only is something actually triggers it -- which might be a config-time warning, or in the likely case that there isn't one, during the first test setup. We should maybe consider doing a "pre-check" so it's always triggered consistently and then can be assumed to be valid. Though it will slow down startup a tiny bit...

  4. It would be nice (though not absolutely necessary) to format the exception a little more nicely, like a ConftestImportFailure is formatted for example. Might not be worth the effort though...

When I get some time I'll try to provide some pointers if needed.

@charlesaracil-ulti
Copy link
Contributor Author

Thanks @bluetech!
I'll be able to work on that next Friday, although I'm quite new to this project and I'll be happy to have some pointers anyway :)

@charlesaracil-ulti
Copy link
Contributor Author

Didn't have as many time today as I expected, still addressed your 1. to get one thing done, will get back to it asap

Base automatically changed from master to main March 9, 2021 20:40
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Oct 21, 2021
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Oct 21, 2021
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Oct 21, 2021
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Oct 21, 2021
nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

warnings: better handle errors raised while applying filterwarnings
3 participants