Opened 21 months ago

Last modified 19 months ago

#6348 enhancement new

trial doesn't show `DeprecationWarnings` by default on 2.7

Reported by: tom.prince Owned by:
Priority: high Milestone:
Component: trial Keywords:
Cc: jml, hs@… Branch: branches/show-all-warnings-6348
(diff, github, buildbot, log)
Author: Launchpad Bug:

Description

Since python 2.7 switch to ignoring DeprecationWarnings by default.
We should make trial enable them (see here for an example of where somebody assumed it did, already).

Change History (10)

comment:1 Changed 21 months ago by DefaultCC Plugin

  • Cc jml added

comment:2 Changed 21 months ago by glyph

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.

comment:3 Changed 21 months ago by glyph

  • Priority changed from normal to 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 20 months ago by Julian

  • Owner set to Julian

comment:5 follow-up: Changed 20 months ago by 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?

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 in reply to: ↑ 5 Changed 20 months ago by glyph

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 19 months ago by hynek

  • Cc hs@… added

comment:8 Changed 19 months ago by Julian

  • Owner Julian deleted

comment:9 Changed 19 months ago by exarkun

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 19 months ago by tom.prince

  • Branch set to 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).

Note: See TracTickets for help on using tickets.