Merge lp:~jml/launchpad/show-warnings-once into lp:launchpad

Proposed by Jonathan Lange
Status: Merged
Merged at revision: 11308
Proposed branch: lp:~jml/launchpad/show-warnings-once
Merge into: lp:launchpad
Diff against target: 32 lines (+6/-3)
1 file modified
lib/lp/scripts/utilities/warninghandler.py (+6/-3)
To merge this branch: bzr merge lp:~jml/launchpad/show-warnings-once
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+31831@code.launchpad.net

Commit message

Only show warnings once at the end of a test run.

Description of the change

It turns out that we blat all over Python's normal warning filtering and show every instance of every warning. This sucks *and* blows at the same time, so I've cobbled together a crappy little hack to fix it.

It's pretty hard to verify that this hack works, so I'm running the branch through ec2 test now.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

If it works, rs=me

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Because the warnings include repr()'s this doesn't help in a lot of situations, e.g.:

/var/launchpad/test/lib/lp/soyuz/tests/test_publishing.py:229: ShouldThisBeUsingRemoveSecurityProxy: removeSecurityProxy(<PackageUpload at 0x16f126d0>) called. Is this correct? Either call it directly or fix the test.
  package_upload)

/var/launchpad/test/lib/lp/soyuz/tests/test_publishing.py:229: ShouldThisBeUsingRemoveSecurityProxy: removeSecurityProxy(<PackageUpload at 0x146b2210>) called. Is this correct? Either call it directly or fix the test.
  package_upload)

I guess we should make sure to improve our __repr__ methods to fix more of these.

Either way, it's better than using a list. Should it perhaps be sorted so we see the warnings grouped per file, line?

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/scripts/utilities/warninghandler.py'
2--- lib/lp/scripts/utilities/warninghandler.py 2010-07-27 13:56:42 +0000
3+++ lib/lp/scripts/utilities/warninghandler.py 2010-08-05 15:19:50 +0000
4@@ -147,7 +147,9 @@
5
6 need_page_titles = []
7 no_order_by = []
8-other_warnings = []
9+
10+# Maps (category, filename, lineno) to WarningReport
11+other_warnings = {}
12
13 old_show_warning = warnings.showwarning
14 def launchpad_showwarning(message, category, filename, lineno, file=None,
15@@ -177,7 +179,8 @@
16 WarningReport(warning_message, important_info)
17 )
18 return
19- other_warnings.append(WarningReport(warning_message, important_info))
20+ other_warnings[(category, filename, lineno)] = WarningReport(
21+ warning_message, important_info)
22
23 def report_need_page_titles():
24 global need_page_titles
25@@ -202,7 +205,7 @@
26 if other_warnings:
27 print
28 print "General warnings."
29- for warninginfo in other_warnings:
30+ for warninginfo in other_warnings.itervalues():
31 print
32 print warninginfo
33