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

No way to write unit tests for warnings #48030

Closed
exarkun mannequin opened this issue Sep 4, 2008 · 8 comments
Closed

No way to write unit tests for warnings #48030

exarkun mannequin opened this issue Sep 4, 2008 · 8 comments
Labels
release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@exarkun
Copy link
Mannequin

exarkun mannequin commented Sep 4, 2008

BPO 3780
Nosy @brettcannon, @benjaminp, @glyph

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2008-09-04.22:48:08.901>
created_at = <Date 2008-09-04.21:59:46.566>
labels = ['invalid', 'type-bug', 'library', 'release-blocker']
title = 'No way to write unit tests for warnings'
updated_at = <Date 2008-09-05.03:39:46.830>
user = 'https://bugs.python.org/exarkun'

bugs.python.org fields:

activity = <Date 2008-09-05.03:39:46.830>
actor = 'brett.cannon'
assignee = 'none'
closed = True
closed_date = <Date 2008-09-04.22:48:08.901>
closer = 'glyph'
components = ['Library (Lib)']
creation = <Date 2008-09-04.21:59:46.566>
creator = 'exarkun'
dependencies = []
files = []
hgrepos = []
issue_num = 3780
keywords = []
message_count = 8.0
messages = ['72530', '72532', '72536', '72540', '72541', '72548', '72553', '72562']
nosy_count = 4.0
nosy_names = ['brett.cannon', 'exarkun', 'benjamin.peterson', 'glyph']
pr_nums = []
priority = 'release blocker'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue3780'
versions = ['Python 2.6']

@exarkun
Copy link
Mannequin Author

exarkun mannequin commented Sep 4, 2008

In Python 2.5 and earlier, the warnings.warn_explicit function could
be replaced in order to test what warnings were emitted by some code.
This allowed unit tests for warnings to be written. Since much of the
warnings module was re-implemented in C, replacing
warnings.warn_explicit no longer has any effect. This, together with
the fact that there are no other public APIs for hooking into the
warning system with duplication suppression disabled means that there is
no reliable way to write unit tests for warnings.

This was previously discussed on python-dev, but no conclusion was
reached. See
http://mail.python.org/pipermail/python-dev/2008-June/080635.html and
the follow-ups.

For Twisted, the consequence of this is roughly 79 failing unit tests in
Python 2.6b3 and no clear way to fix them, except to stop using the
standard library warnings module.

@exarkun exarkun mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 4, 2008
@benjaminp
Copy link
Contributor

Would the new catch_warnings [1] context manager help in this regard?

[1] http://docs.python.org/dev/library/warnings.html#warnings.catch_warnings

@brettcannon
Copy link
Member

It sounds like you are trying to get around "once"/"default" rules to
see all warnings raised. Why can't you use catch_warnings() and do
simplefilter("always") or use "error"? Otherwise you can force the
importing and use of the pure Python implementation of warnings if you
really want to continue to use your hacked version of warn_explicit (see
test_warnings on how to control whether the C implementation gets used
or not).

@exarkun
Copy link
Mannequin Author

exarkun mannequin commented Sep 4, 2008

I was aware of it, but I didn't realize adding an "always" filter would
make sure all warnings always got noticed. I haven't tried changing
Twisted's use of the warnings module yet, but it looks like
catch_warnings will work here.

@glyph
Copy link
Mannequin

glyph mannequin commented Sep 4, 2008

Looks like we just misunderstood the way the warnings filter works, and
catch_warnings' interaction with it. Thanks for the priority bump,
guido, and sorry for the false alarm!

@glyph glyph mannequin closed this as completed Sep 4, 2008
@glyph glyph mannequin added the invalid label Sep 4, 2008
@brettcannon
Copy link
Member

On Thu, Sep 4, 2008 at 3:48 PM, Glyph Lefkowitz <report@bugs.python.org> wrote:

Glyph Lefkowitz <glyph@divmod.com> added the comment:

Looks like we just misunderstood the way the warnings filter works, and
catch_warnings' interaction with it. Thanks for the priority bump,
guido, and sorry for the false alarm!

If there is a doc problem, please suggest some better wording! I
tossed the text together the best I could but if it needs improvement
I welcome suggestions.

@glyph
Copy link
Mannequin

glyph mannequin commented Sep 4, 2008

The use of the term "filter" is pretty confusing. I would normally say
it was just me, but a bunch of other Twisted hackers seemed to interpret
it the same way: a "filter" is something that has an opportunity to
remove something else. For example, the python builtin function
"filter". Experimentation with the filters list seems to confirm this
impression, since later filters do not have an opportunity to access the
warnings that earlier filters have removed. The intuitive leap there is
to assume that inserting a filter at the head of the list won't do
anything different than inserting it at the tail, since a later filter
will remove it.

I can't think of an obvious recommendation to improve the text for the
filter system itself, because upon reading it in more depth, it's fairly
clear. Maybe the heading could be changed to something more like
"intercepting warnings" or "controlling the way that warnings are
emitted"? Something attention-grabbing that describes its purpose and
doesn't use the word "filter". Even a sub-heading which simply
described how to use 'always' filter to cause every warning to be
emitted would be helpful.

The biggest improvement to the documentation for "catch_warnings" would
be to put it in a section of its own, "How To Test Your Code Which Uses
Warnings", not as a footnote in "Available Classes".

(Also, as a minor note to be taken at your discretion: should
catch_warnings be named PEP-8 style as a class, since it is one? I
don't really want to open that can of worms after reading the
interminable threading.Event thread, but it seemed worth saying as long
as was talking about this...)

@brettcannon
Copy link
Member

On Thu, Sep 4, 2008 at 4:18 PM, Glyph Lefkowitz <report@bugs.python.org> wrote:

Glyph Lefkowitz <glyph@divmod.com> added the comment:

The use of the term "filter" is pretty confusing. I would normally say
it was just me, but a bunch of other Twisted hackers seemed to interpret
it the same way: a "filter" is something that has an opportunity to
remove something else. For example, the python builtin function
"filter". Experimentation with the filters list seems to confirm this
impression, since later filters do not have an opportunity to access the
warnings that earlier filters have removed. The intuitive leap there is
to assume that inserting a filter at the head of the list won't do
anything different than inserting it at the tail, since a later filter
will remove it.

OK, so you were thinking of the filter list in a functional sense
where exceptions are passed down the chain to each filter rule instead
of a bunch of tests that stop checking once one of the rules applies.

I can't think of an obvious recommendation to improve the text for the
filter system itself, because upon reading it in more depth, it's fairly
clear. Maybe the heading could be changed to something more like
"intercepting warnings" or "controlling the way that warnings are
emitted"? Something attention-grabbing that describes its purpose and
doesn't use the word "filter". Even a sub-heading which simply
described how to use 'always' filter to cause every warning to be
emitted would be helpful.

OK.

The biggest improvement to the documentation for "catch_warnings" would
be to put it in a section of its own, "How To Test Your Code Which Uses
Warnings", not as a footnote in "Available Classes".

Fair enough.

(Also, as a minor note to be taken at your discretion: should
catch_warnings be named PEP-8 style as a class, since it is one? I
don't really want to open that can of worms after reading the
interminable threading.Event thread, but it seemed worth saying as long
as was talking about this...)

It's partially historical since test.test_support.catch_warning() is a
context manager created from a generator. But it's also because I want
to have the option to change the context manager in the future if the
bootstrapping reasons that led to it being a class can go away. I
should probably change the section title to "Available Context
Managers" so people don't start making assumptions.

-Brett

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants