Merge lp:~frankban/launchpad/bug-1010251 into lp:launchpad

Proposed by Francesco Banconi
Status: Merged
Approved by: Francesco Banconi
Approved revision: no longer in the source branch.
Merged at revision: 15392
Proposed branch: lp:~frankban/launchpad/bug-1010251
Merge into: lp:launchpad
Diff against target: 44 lines (+11/-7)
1 file modified
buildout-templates/bin/test.in (+11/-7)
To merge this branch: bzr merge lp:~frankban/launchpad/bug-1010251
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+109627@code.launchpad.net

Commit message

Updated bin/test to change working dir before importing from site. Fixed bug 1010251.

Description of the change

= Summary =

test_archive_webservice.TestArchiveWebservice.test_checkUpload_bad_pocket fails rarely/intermittently in parallel tests.
I was able to reproduce the bug only running the test in isolation inside an ephemeral, e.g.::

     sudo lxc-start-ephemeral -u frankban -o lp -- "xvfb-run --error-file=/var/tmp/xvfb-errors.log --server-args='-screen 0 1024x768x24' -a $PWD/bin/test -cvvt lp.soyuz.browser.tests.test_archive_webservice.TestArchiveWebservice.test_checkUpload_bad_pocket"

The css path used by the docutils html writer in this case is relative to $PWD, not relative to the branch root.
Indeed I was able to reproduce the problem without ephemerals too, calling bin/test from a different working dir (e.g. my home dir).

I've seen that, when run in isolation, the test uses the writer indirectly instantiated by `import site` inside bin/test BEFORE the script itself changes the working dir. Vice versa, when runned normally, the test is usually run in a subprocess, so the docutils writer is re-initialized with the correct working dir.

== Proposed fix ==

In bin/test, change CWD before `import site`.

== Implementation details ==

See Proposed fix.

== Tests ==

cd
launchpad/lp-branches/bug-1010251/bin/test -cvvt lp.soyuz.browser.tests.test_archive_webservice.TestArchiveWebservice.test_checkUpload_bad_pocket

NO QA

== lint ==

Linting changed files:
  buildout-templates/bin/test.in

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'buildout-templates/bin/test.in'
2--- buildout-templates/bin/test.in 2012-06-07 10:03:44 +0000
3+++ buildout-templates/bin/test.in 2012-06-11 13:12:21 +0000
4@@ -22,6 +22,17 @@
5
6 # Initialize our paths.
7 ${python-relative-path-setup}
8+
9+
10+# The working directory change is just so that the test script
11+# can be invoked from places other than the root of the source
12+# tree. This is very useful for IDE integration, so an IDE can
13+# e.g. run the test that you are currently editing.
14+BUILD_DIR = ${buildout:directory|path-repr}
15+there = os.getcwd()
16+os.chdir(BUILD_DIR)
17+
18+
19 import sys
20 sys.path.insert(0, ${scripts:parts-directory|path-repr})
21 import site
22@@ -42,7 +53,6 @@
23 doctest._SpoofOut = _SpoofOut
24
25
26-BUILD_DIR = ${buildout:directory|path-repr}
27 CUSTOM_SITE_DIR = ${scripts:parts-directory|path-repr}
28
29 # Make tests run in a timezone no launchpad developers live in.
30@@ -250,14 +260,8 @@
31 if local_options.verbose >= 3 and main_process:
32 profiled.setup_profiling()
33
34- # The working directory change is just so that the test script
35- # can be invoked from places other than the root of the source
36- # tree. This is very useful for IDE integration, so an IDE can
37- # e.g. run the test that you are currently editing.
38 try:
39 try:
40- there = os.getcwd()
41- os.chdir(BUILD_DIR)
42 testrunner.run([])
43 except SystemExit:
44 # Print Layer profiling report if requested.