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
=== modified file 'Makefile'
--- Makefile 2010-05-12 09:27:10 +0000
+++ Makefile 2010-05-17 18:35:44 +0000
@@ -67,6 +67,7 @@
6767
68apidoc: compile $(API_INDEX)68apidoc: compile $(API_INDEX)
6969
70# Run by PQM.
70check_merge: $(PY)71check_merge: $(PY)
71 [ `PYTHONPATH= bzr status -S database/schema/ | \72 [ `PYTHONPATH= bzr status -S database/schema/ | \
72 grep -v "\(^P\|pending\|security.cfg\|Makefile\|unautovacuumable\|_pythonpath.py\)" | wc -l` -eq 0 ]73 grep -v "\(^P\|pending\|security.cfg\|Makefile\|unautovacuumable\|_pythonpath.py\)" | wc -l` -eq 0 ]
@@ -86,7 +87,19 @@
86check: clean build87check: clean build
87 # Run all tests. test_on_merge.py takes care of setting up the88 # Run all tests. test_on_merge.py takes care of setting up the
88 # database.89 # database.
89 ${PY} -t ./test_on_merge.py $(VERBOSITY)90 ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS)
91
92# A version of 'make check' that applies modifications specifically for the
93# ec2 environment.
94ec2_check: clean build
95 # XXX mars 2010-05-17 bug=570380
96 # Disable the windmill test suite to prevent the whole system from
97 # hanging. See bug 570380.
98 #
99 # Yes, this is code duplication. If you conceive of a better
100 # solution in a less heated moment, then please replace this!
101 ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS) \
102 --layer='!WindmillLayer'
90103
91jscheck: build104jscheck: build
92 # Run all JavaScript integration tests. The test runner takes care of105 # Run all JavaScript integration tests. The test runner takes care of
@@ -94,7 +107,7 @@
94 @echo107 @echo
95 @echo "Running the JavaScript integration test suite"108 @echo "Running the JavaScript integration test suite"
96 @echo109 @echo
97 bin/test $(VERBOSITY) --layer=WindmillLayer110 bin/test $(VERBOSITY) $(TESTOPTS) --layer=WindmillLayer
98111
99jscheck_functest: build112jscheck_functest: build
100 # Run the old functest Windmill integration tests. The test runner113 # Run the old functest Windmill integration tests. The test runner
@@ -107,7 +120,8 @@
107check_mailman: build120check_mailman: build
108 # Run all tests, including the Mailman integration121 # Run all tests, including the Mailman integration
109 # tests. test_on_merge.py takes care of setting up the database.122 # tests. test_on_merge.py takes care of setting up the database.
110 ${PY} -t ./test_on_merge.py $(VERBOSITY) --layer=MailmanLayer123 ${PY} -t ./test_on_merge.py $(VERBOSITY) $(TESTOPTS) \
124 --layer=MailmanLayer
111125
112lint: ${PY}126lint: ${PY}
113 @bash ./bin/lint.sh127 @bash ./bin/lint.sh
@@ -437,7 +451,7 @@
437 mv lp-clustered.svg.tmp lp-clustered.svg451 mv lp-clustered.svg.tmp lp-clustered.svg
438452
439PYDOCTOR = pydoctor453PYDOCTOR = pydoctor
440PYDOCTOR_OPTIONS = 454PYDOCTOR_OPTIONS =
441455
442pydoctor:456pydoctor:
443 $(PYDOCTOR) --make-html --html-output=apidocs --add-package=lib/lp \457 $(PYDOCTOR) --make-html --html-output=apidocs --add-package=lib/lp \
@@ -447,7 +461,7 @@
447461
448.PHONY: apidoc check tags TAGS zcmldocs realclean clean debug stop\462.PHONY: apidoc check tags TAGS zcmldocs realclean clean debug stop\
449 start run ftest_build ftest_inplace test_build test_inplace pagetests\463 start run ftest_build ftest_inplace test_build test_inplace pagetests\
450 check check_merge \464 check check_merge ec2_check \
451 schema default launchpad.pot check_merge_ui pull scan sync_branches\465 schema default launchpad.pot check_merge_ui pull scan sync_branches\
452 reload-apache hosted_branches check_db_merge check_mailman check_config\466 reload-apache hosted_branches check_db_merge check_mailman check_config\
453 jsbuild jsbuild_lazr clean_js buildonce_eggs \467 jsbuild jsbuild_lazr clean_js buildonce_eggs \
454468
=== modified file 'lib/devscripts/ec2test/remote.py'
--- lib/devscripts/ec2test/remote.py 2010-04-29 15:07:51 +0000
+++ lib/devscripts/ec2test/remote.py 2010-05-17 18:35:44 +0000
@@ -277,7 +277,7 @@
277277
278 def build_test_command(self):278 def build_test_command(self):
279 """See BaseTestRunner.build_test_command()."""279 """See BaseTestRunner.build_test_command()."""
280 command = ['make', 'check', 'VERBOSITY=' + self.test_options]280 command = ['make', 'ec2_check', 'TESTOPTS=' + self.test_options]
281 return command281 return command
282282
283283