Launchpad itself

Merge lp:~maxb/launchpad/ignored-asserts into lp:launchpad

  1. ignored-asserts
  2. Merge into devel
Proposed by Max Bowsher
Status: Merged
Merged at revision: not available
Proposed branch: lp:~maxb/launchpad/ignored-asserts
Merge into: lp:launchpad
Diff against target: 99 lines (+21/-8)
5 files modified
lib/lp/soyuz/doc/initialise-from-parent.txt (+13/-0)
lib/lp/translations/model/translatablemessage.py (+1/-1)
lib/lp/translations/utilities/rosettastats.py (+1/-1)
lib/lp/translations/utilities/translation_import.py (+2/-2)
scripts/ftpmaster-tools/initialise-from-parent.py (+4/-4)
To merge this branch: bzr merge lp:~maxb/launchpad/ignored-asserts
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Graham Binns (community) code Needs Fixing
Review via email: mp+18889@code.launchpad.net

Commit message

Fix assert statements which were not being tested owing to misplaced parentheses.

To post a comment you must log in.
Revision history for this message
Max Bowsher (maxb) wrote :

The Python assert statement is not a function, and treating it as such - i.e.:

   assert (a == b, "The things weren't equal!")

results in evaluating a 2-element tuple as a boolean, thus, it's approximately the same as 'assert True', i.e. a no-op.

Python 2.6 recognizes this flaw, and adds a SyntaxWarning to report this case. Thus, by running:

   find -name \*.py -type f | xargs python2.6 -m py_compile

I've located all instances of the problem in the Launchpad source.

Here's a branch applying the requisite syntax fixes.

Revision history for this message
Max Bowsher (maxb) wrote :

AssertionError: Parent must not have PENDING builds is triggering in lib/lp/soyuz/tests/../doc/initialise-from-parent.txt

Revision history for this message
Graham Binns (gmb) wrote :

Hi Max,

Nice catch! I like it. You should add this check to bin/lint.sh so that make lint will pick up this error for us.

I'm marking this needs fixing but only because I want the change to the linter in there as well.

review: Needs Fixing (code)
Revision history for this message
Max Bowsher (maxb) wrote :

> Nice catch! I like it. You should add this check to bin/lint.sh so that make
> lint will pick up this error for us.

Graham,

Are you sure you really want bin/lint.sh to depend on having a python2.6 installed?

In any case, once Launchpad properly migrates to Python 2.6 in the future, Python itself will be linting this.

Though, I guess this is "Needs fixing" anyway, since we need to resolve that Soyuz test failure before this branch can land. Should we aim to fix the soyuz test first, or to land this branch with the soyuz asserts commented out and XXXed?

Revision history for this message
Graham Binns (gmb) wrote :

On 9 February 2010 11:26, Max Bowsher <email address hidden> wrote:
>> Nice catch! I like it. You should add this check to bin/lint.sh so that make
>> lint will pick up this error for us.
>
> Graham,
>
> Are you sure you really want bin/lint.sh to depend on having a python2.6 installed?
>

Ah, good point. I forget sometimes that not everyone runs on Karmic.
However, we could have a conditional that runs the check if 2.6 is
available.

> In any case, once Launchpad properly migrates to Python 2.6 in the future, Python itself will be linting this.
>

True. Do we have a timetable for that? All the ML threads I've seen
are vague about it.

> Though, I guess this is "Needs fixing" anyway, since we need to resolve that Soyuz test failure before this branch can land. Should we aim to fix the soyuz test first, or to land this branch with the soyuz asserts commented out and XXXed?

We should fix the test, certainly. If the test is very broken, you
should speak to Julian about it.

Revision history for this message
Max Bowsher (maxb) wrote :

> On 9 February 2010 11:26, Max Bowsher wrote:
> >> Nice catch! I like it. You should add this check to bin/lint.sh so that
> >> make lint will pick up this error for us.
> >
> > Are you sure you really want bin/lint.sh to depend on having a python2.6
> > installed?
>
> Ah, good point. I forget sometimes that not everyone runs on Karmic.
> However, we could have a conditional that runs the check if 2.6 is
> available.
>
> > In any case, once Launchpad properly migrates to Python 2.6 in the future,
> > Python itself will be linting this.
>
> True. Do we have a timetable for that? All the ML threads I've seen
> are vague about it.

Let's not bother with adding anything to lint.sh, since the Python 2.6 migration seems to be getting under way with Salgado doing some work on it, and the impending release of Lucid means it will become a high priority.

> > Though, I guess this is "Needs fixing" anyway, since we need to resolve that
> > Soyuz test failure before this branch can land. Should we aim to fix the soyuz
> > test first, or to land this branch with the soyuz asserts commented out and
> > XXXed?
>
> We should fix the test, certainly. If the test is very broken, you
> should speak to Julian about it.

I have now fixed the test, thanks to guidance from wgrant.

Ready for re-review and landing.

Revision history for this message
Abel Deuring (adeuring) :
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/soyuz/doc/initialise-from-parent.txt'
2--- lib/lp/soyuz/doc/initialise-from-parent.txt 2009-11-14 02:55:15 +0000
3+++ lib/lp/soyuz/doc/initialise-from-parent.txt 2010-04-09 11:45:42 +0000
4@@ -23,6 +23,19 @@
5 ... 'The Foobuntu', 'yeck', 'doom',
6 ... '888', hoary, hoary.owner)
7
8+
9+The script will check that there are no NEEDSBUILD builds in the parent
10+distroseries' release pocket, so we need to tweak the status of the NEEDSBUILD
11+builds that exist in Ubuntu Hoary in the sampledata:
12+
13+ >>> from lp.buildmaster.interfaces.buildbase import BuildStatus
14+ >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
15+
16+ >>> pending_builds = hoary.getBuildRecords(BuildStatus.NEEDSBUILD,
17+ ... pocket=PackagePublishingPocket.RELEASE)
18+ >>> for build in pending_builds:
19+ ... build.buildstate = BuildStatus.FAILEDTOBUILD
20+
21 >>> import transaction
22 >>> transaction.commit()
23
24
25=== modified file 'lib/lp/translations/model/translatablemessage.py'
26--- lib/lp/translations/model/translatablemessage.py 2009-08-26 16:24:02 +0000
27+++ lib/lp/translations/model/translatablemessage.py 2010-04-09 11:45:42 +0000
28@@ -34,7 +34,7 @@
29 Both potmsgset and pofile must be related, meaning they refer to the
30 same `IPOTemplate` instance.
31 """
32- assert (pofile.potemplate.getPOTMsgSetByID(potmsgset.id) != None,
33+ assert pofile.potemplate.getPOTMsgSetByID(potmsgset.id) != None, (
34 "POTMsgSet and POFile must refer to the same POTemplate.")
35
36 self.potmsgset = potmsgset
37
38=== modified file 'lib/lp/translations/utilities/rosettastats.py'
39--- lib/lp/translations/utilities/rosettastats.py 2009-07-17 00:26:05 +0000
40+++ lib/lp/translations/utilities/rosettastats.py 2010-04-09 11:45:42 +0000
41@@ -62,7 +62,7 @@
42 """See IRosettaStats."""
43 untranslated = self.messageCount() - self.translatedCount(language)
44 # Statistics should not be ever less than 0
45- assert (untranslated >= 0,
46+ assert untranslated >= 0, (
47 'Stats error in %r id %d, %d untranslated' % (
48 self, self.id, untranslated))
49 return untranslated
50
51=== modified file 'lib/lp/translations/utilities/translation_import.py'
52--- lib/lp/translations/utilities/translation_import.py 2010-02-23 08:25:39 +0000
53+++ lib/lp/translations/utilities/translation_import.py 2010-04-09 11:45:42 +0000
54@@ -595,7 +595,7 @@
55 def __init__(self, translation_import_queue_entry, importer, logger):
56 """Construct an Importer for a translation template."""
57
58- assert(translation_import_queue_entry.pofile is None,
59+ assert translation_import_queue_entry.pofile is None, (
60 "Pofile must be None when importing a template.")
61
62 # Call base constructor
63@@ -671,7 +671,7 @@
64 def __init__(self, translation_import_queue_entry, importer, logger):
65 """Construct an Importer for a translation file."""
66
67- assert(translation_import_queue_entry.pofile is not None,
68+ assert translation_import_queue_entry.pofile is not None, (
69 "Pofile must not be None when importing a translation.")
70
71 # Call base constructor
72
73=== modified file 'scripts/ftpmaster-tools/initialise-from-parent.py'
74--- scripts/ftpmaster-tools/initialise-from-parent.py 2010-03-19 21:26:13 +0000
75+++ scripts/ftpmaster-tools/initialise-from-parent.py 2010-04-09 11:45:42 +0000
76@@ -117,7 +117,7 @@
77 pending_builds = parentseries.getBuildRecords(
78 BuildStatus.NEEDSBUILD, pocket=PackagePublishingPocket.RELEASE)
79
80- assert (pending_builds.count() == 0,
81+ assert pending_builds.count() == 0, (
82 'Parent must not have PENDING builds')
83
84 def check_queue(distroseries):
85@@ -143,11 +143,11 @@
86 PackageUploadStatus.UNAPPROVED,
87 pocket=PackagePublishingPocket.RELEASE)
88
89- assert (new_items.count() == 0,
90+ assert new_items.count() == 0, (
91 'Parent NEW queue must be empty')
92- assert (accepted_items.count() == 0,
93+ assert accepted_items.count() == 0, (
94 'Parent ACCEPTED queue must be empty')
95- assert (unapproved_items.count() == 0,
96+ assert unapproved_items.count() == 0, (
97 'Parent UNAPPROVED queue must be empty')
98
99 def copy_architectures(distroseries):