launchpad-reviewers team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/launchpad/bug-1010251 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1010251 in Launchpad itself: "lp.soyuz.browser.tests.test_archive_webservice.TestArchiveWebservice.test_checkUpload_bad_pocket fails rarely/intermittently in parallel tests"
  https://bugs.launchpad.net/launchpad/+bug/1010251

For more details, see:
https://code.launchpad.net/~frankban/launchpad/bug-1010251/+merge/109627

= 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

-- 
https://code.launchpad.net/~frankban/launchpad/bug-1010251/+merge/109627
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~frankban/launchpad/bug-1010251 into lp:launchpad.

=== modified file 'buildout-templates/bin/test.in'
--- buildout-templates/bin/test.in	2012-06-07 10:03:44 +0000
+++ buildout-templates/bin/test.in	2012-06-11 13:12:21 +0000
@@ -22,6 +22,17 @@
 
 # Initialize our paths.
 ${python-relative-path-setup}
+
+
+# The working directory change is just so that the test script
+# can be invoked from places other than the root of the source
+# tree. This is very useful for IDE integration, so an IDE can
+# e.g. run the test that you are currently editing.
+BUILD_DIR = ${buildout:directory|path-repr}
+there = os.getcwd()
+os.chdir(BUILD_DIR)
+
+
 import sys
 sys.path.insert(0, ${scripts:parts-directory|path-repr})
 import site
@@ -42,7 +53,6 @@
 doctest._SpoofOut = _SpoofOut
 
 
-BUILD_DIR = ${buildout:directory|path-repr}
 CUSTOM_SITE_DIR = ${scripts:parts-directory|path-repr}
 
 # Make tests run in a timezone no launchpad developers live in.
@@ -250,14 +260,8 @@
     if local_options.verbose >= 3 and main_process:
         profiled.setup_profiling()
 
-    # The working directory change is just so that the test script
-    # can be invoked from places other than the root of the source
-    # tree. This is very useful for IDE integration, so an IDE can
-    # e.g. run the test that you are currently editing.
     try:
         try:
-            there = os.getcwd()
-            os.chdir(BUILD_DIR)
             testrunner.run([])
         except SystemExit:
             # Print Layer profiling report if requested.