Launchpad itself

Merge lp:~mars/launchpad/disable-windmill-tests into lp:launchpad

  1. disable-windmill-tests
  2. Merge into devel
Proposed by Māris Fogels
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: 10873
Proposed branch: lp:~mars/launchpad/disable-windmill-tests
Merge into: lp:launchpad
Diff against target: 80 lines (+20/-6)
2 files modified
Makefile (+19/-5)
lib/devscripts/ec2test/remote.py (+1/-1)
To merge this branch: bzr merge lp:~mars/launchpad/disable-windmill-tests
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+25457@code.launchpad.net

Commit message

This branch shuts off the test suite WindmillLayer when running on ec2. Works around bug 570380.

Description of the change

Hi,

This branch shuts off the test suite WindmillLayer when running on ec2. This will hopefully work around the ec2 test suite hangs (bug 570380).

The branch works by adding a new ec2_check target to the project Makefile, then having the ec2test remote component run the new target. I also included some Makefile variable cleanups and comments in this branch, particularly to remove ec2test's use of the overloaded Makefile VERBOSITY variable.

Pre-implementation call with: BjornT
Lint: None
Tested: Locally with "make ec2_check TESTOPTS='-t test_bug_also_affects_picker'". ec2 tests are pending.

To post a comment you must log in.
Revision history for this message
Māris Fogels (mars) wrote :

Tested with: utilities/ec2 test -o '-t test_bug_also_affects_picker'

Works as designed. Windmill layer tests are skipped.

Revision history for this message
Gary Poster (gary) wrote :

[3:03pm] gary_poster: mars, you added in TESTOPTS for theoretical correctness, yes?
[3:03pm] gary_poster: that is, no one is actually using them, it seems?
[3:03pm] mars: gary_poster, it was already in there, just not used properly.
[3:04pm] mars: they are used by three targets lower down in the file
[3:04pm] gary_poster: ah, gotcha mars
[3:04pm] mars: I was going to rename VERBOSITY to TESTOPTS myself, and was surprised to find TESTOPTS was already in there
[3:05pm] gary_poster: yeah, another approach would be to rip it out, but I suppose this is safer
[3:05pm] mars: using the value of the VERBOSITY variable to pass "-vvv -t --layer" to the test process really rubbed me the wrong way. Refactoring gene I guess
[3:05pm] mars: gary_poster, safer, yes. I don't know who else used VERBOSITY that way.
[3:06pm] gary_poster: mars, have you checked to see what happens to the test runner if you say --layer='!WindmillLayer' --layer='WindmillLayer'?
[3:06pm] mars: Needed a fast fix. This is one step forward.
[3:06pm] mars: My guess is that --layer=!Layer --layer=Layer *will* execute the desired test suite.
[3:06pm] gary_poster: well, for a fast fix, maybe leaving TESTOPTS out of the picture would be wiser. Otherwise I have questions about it
[3:06pm] mars: But no, I have not explicitly tested that
[3:07pm] mars: gary_poster, oh? What questions?
[3:07pm] gary_poster: for instance--if you are right (which would be my guess) then I would suggest that ``bin/test $(VERBOSITY) $(TESTOPTS) --layer=WindmillLayer`` should instead be ``bin/test $(VERBOSITY) --layer=WindmillLayer $(TESTOPTS)`` so that TESTOPTS can actually override things
[3:07pm] mars: gary_poster, I treat TESTOPTS like CFLAGS. VERBOSITY is independent of the two.
[3:08pm] mars: gary_poster, ok, interesting. That woudl have come up regardless if I used VERBOSITY or TESTOPTS.
[3:09pm] gary_poster: yeah, calling it TESTOPTS is more correct, and makes the possible problem clearer.
[3:09pm] gary_poster: mars, I'll approve, simply asking that you consider my question...possibly later.
[3:09pm] mars: +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2010-05-12 09:27:10 +0000
3+++ Makefile 2010-05-17 18:35:44 +0000
4@@ -67,6 +67,7 @@
5
6 apidoc: compile $(API_INDEX)
7
8+# Run by PQM.
9 check_merge: $(PY)
10 [ `PYTHONPATH= bzr status -S database/schema/ | \
11 grep -v "\(^P\|pending\|security.cfg\|Makefile\|unautovacuumable\|_pythonpath.py\)" | wc -l` -eq 0 ]
12@@ -86,7 +87,19 @@
13 check: clean build
14 # Run all tests. test_on_merge.py takes care of setting up the
15 # database.
16- ${PY} -t ./test_on_merge.py $(VERBOSITY)
17+ ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS)
18+
19+# A version of 'make check' that applies modifications specifically for the
20+# ec2 environment.
21+ec2_check: clean build
22+ # XXX mars 2010-05-17 bug=570380
23+ # Disable the windmill test suite to prevent the whole system from
24+ # hanging. See bug 570380.
25+ #
26+ # Yes, this is code duplication. If you conceive of a better
27+ # solution in a less heated moment, then please replace this!
28+ ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS) \
29+ --layer='!WindmillLayer'
30
31 jscheck: build
32 # Run all JavaScript integration tests. The test runner takes care of
33@@ -94,7 +107,7 @@
34 @echo
35 @echo "Running the JavaScript integration test suite"
36 @echo
37- bin/test $(VERBOSITY) --layer=WindmillLayer
38+ bin/test $(VERBOSITY) $(TESTOPTS) --layer=WindmillLayer
39
40 jscheck_functest: build
41 # Run the old functest Windmill integration tests. The test runner
42@@ -107,7 +120,8 @@
43 check_mailman: build
44 # Run all tests, including the Mailman integration
45 # tests. test_on_merge.py takes care of setting up the database.
46- ${PY} -t ./test_on_merge.py $(VERBOSITY) --layer=MailmanLayer
47+ ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS) \
48+ --layer=MailmanLayer
49
50 lint: ${PY}
51 @bash ./bin/lint.sh
52@@ -437,7 +451,7 @@
53 mv lp-clustered.svg.tmp lp-clustered.svg
54
55 PYDOCTOR = pydoctor
56-PYDOCTOR_OPTIONS =
57+PYDOCTOR_OPTIONS =
58
59 pydoctor:
60 $(PYDOCTOR) --make-html --html-output=apidocs --add-package=lib/lp \
61@@ -447,7 +461,7 @@
62
63 .PHONY: apidoc check tags TAGS zcmldocs realclean clean debug stop\
64 start run ftest_build ftest_inplace test_build test_inplace pagetests\
65- check check_merge \
66+ check check_merge ec2_check \
67 schema default launchpad.pot check_merge_ui pull scan sync_branches\
68 reload-apache hosted_branches check_db_merge check_mailman check_config\
69 jsbuild jsbuild_lazr clean_js buildonce_eggs \
70
71=== modified file 'lib/devscripts/ec2test/remote.py'
72--- lib/devscripts/ec2test/remote.py 2010-04-29 15:07:51 +0000
73+++ lib/devscripts/ec2test/remote.py 2010-05-17 18:35:44 +0000
74@@ -277,7 +277,7 @@
75
76 def build_test_command(self):
77 """See BaseTestRunner.build_test_command()."""
78- command = ['make', 'check', 'VERBOSITY=' + self.test_options]
79+ command = ['make', 'ec2_check', 'TESTOPTS=' + self.test_options]
80 return command
81
82