Opened 9 years ago
Last modified 2 years ago
#6348 enhancement new
trial doesn't show `DeprecationWarnings` by default
Reported by: | Tom Prince | Owned by: | |
---|---|---|---|
Priority: | high | Milestone: | |
Component: | trial | Keywords: | |
Cc: | Jonathan Lange, Hynek Schlawack | Branch: |
branches/show-all-warnings-6348
branch-diff, diff-cov, branch-cov, buildbot |
Author: |
Description (last modified by )
Since python 2.7 switch to ignoring DeprecationWarning
s by default.
We should make trial enable them (see here for an example of where somebody assumed it did, already).
Change History (14)
comment:1 Changed 9 years ago by
Cc: | Jonathan Lange added |
---|
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
Priority: | normal → high |
---|
I'd say this is high priority because I think there has been some confusion around this - I think for multiple reasons, but if for no other reason then because I've been spreading confusion about this because I thought it was already implemented.
comment:4 Changed 9 years ago by
Owner: | set to Julian Berman |
---|
comment:5 follow-up: 6 Changed 9 years ago by
So, I started working on this after thinking trial should do this and then finding this ticket, but after a bit of thought and some work on it I'm not so sure anymore. Trial is already being quite intrusive (which was why I reopened #3070). So, I have what I've done already, but can someone clarify why we should do this when there is already an acceptable way to have this happen for every Python program in existence?
The problem with PYTHONWARNINGS is really nothing other than lack of knowledge (even though it is specifically mentioned in the warnings docs to test your code with warnings turned on.). I'm sure I also have made fun of Python before for having the default be to hide them, but things are as they are and there's an acceptable way to deal with that. So I'm currently of the opinion that this should be wontfixed and #3070 worked on. Do you have reason to disagree?
comment:6 Changed 9 years ago by
Replying to Julian:
So, I started working on this after thinking trial should do this and then finding this ticket, but after a bit of thought and some work on it I'm not so sure anymore. Trial is already being quite intrusive (which was why I reopened #3070). So, I have what I've done already, but can someone clarify why we should do this when there is already an acceptable way to have this happen for every Python program in existence?
Yes. Python implemented warning-hiding for a specific reason: warnings were showing up when users ran applications. Specifically, warnings would show up on terminals when users ran perfectly-functional command-line Python applications, leading users to complain and think that something serious was wrong, when in fact everything was perfectly fine from their perspective.
trial
is not an arbitrary application; it's a testing tool, and different default behavior makes sense.
The problem with PYTHONWARNINGS is really nothing other than lack of knowledge (even though it is specifically mentioned in the warnings docs to test your code with warnings turned on.). I'm sure I also have made fun of Python before for having the default be to hide them, but things are as they are and there's an acceptable way to deal with that. So I'm currently of the opinion that this should be wontfixed and #3070 worked on. Do you have reason to disagree?
Trial is a testing tool, and an implicit part of our compatibility policy.
The gist of Twisted's compatibility is this: if you can run your program's (100% coverage!) test suite with no warnings under trial, then you ought to be able to safely upgrade a single version of Twisted. If trial does not output warnings by default unless you set a special environment variable, then the default behavior is wrong. Python may make you jump through hoops to make things work correctly, but not Twisted :).
I've already given my reasons for not fixing #3070 on that ticket. However, that does not mean that Trial's reporting of warnings should not be improved. For example, warnings ought to show up, like failures and errors, in the summary output. Also, see this ticket about improving trial's other output behaviors to present better-formatted output, which should include warnings.
comment:7 Changed 9 years ago by
Cc: | Hynek Schlawack added |
---|
comment:8 Changed 9 years ago by
Owner: | Julian Berman deleted |
---|
comment:9 Changed 9 years ago by
Heya Julian, I wonder if you figured out why trial's existing warning integration code isn't managing to grab these warnings? If so, it might help the next person who picks up this ticket. If not, no big deal - just wanted to make sure we capture whatever information we have. Thanks!
comment:10 Changed 9 years ago by
Branch: | → branches/show-all-warnings-6348 |
---|
I put some code in the branch that should always show all unflushed warnings.
It does mean that trial will completely ignore any warning setting of the user (such as -Werror
for example).
comment:11 Changed 4 years ago by
Description: | modified (diff) |
---|
comment:12 Changed 4 years ago by
Description: | modified (diff) |
---|
comment:13 Changed 4 years ago by
Well, the next person finding this ticket happens to be me again, since trial is being broken again. Time to go back down the rabbit hole and remember why it's breaking the warnings module again... Maybe this time leave enough info for the next time I find the ticket :/
comment:14 Changed 2 years ago by
Summary: | trial doesn't show `DeprecationWarnings` by default on 2.7 → trial doesn't show `DeprecationWarnings` by default |
---|
dropping "on 2.7" from the description here because my understanding is that this meant "2.7+" and we are well past that world now
OK, so, all Twisted developers should
export PYTHONWARNINGS=all
right now.I think the reason I was confused is that some, but apparently not all of our buildbots pass
-Wall
to trial.