Code review comment for lp://staging/~james-w/pkgme/further-test-fixes

 review approve
 merge approve

=== modified file 'pkgme/backend.py'
--- pkgme/backend.py 2010-12-18 14:55:00 +0000
+++ pkgme/backend.py 2011-01-16 19:09:53 +0000
> @@ -1,3 +1,4 @@
> +import errno
> import os
> import sys
> import subprocess
> @@ -16,10 +17,10 @@
>
>
> def get_backend_dir(underunder_file, backend):
> - backend_dir = os.path.normpath(
> + backend_dir = os.path.abspath(os.path.normpath(

Only abspath() should be necessary, as it calls normpath() on its return
value. See Python's Lib/os/posixpath.py.

> os.path.join(
> os.path.dirname(underunder_file), os.pardir,
> - 'backends', backend))
> + 'backends', backend)))
> # When 'python setup.py test' is run with virtualenv, backend_dir will not
> # point to the right place. It will point into the
> # build/lib.{platform}-{version} directory and that cannot be used as a
> @@ -114,12 +115,19 @@
> self.basepath = basepath
>
> def want(self, path):
> - script_path = os.path.join(self.basepath, self.WANT_SCRIPT_NAME)
> + script_path = os.path.join(
> + os.path.abspath(self.basepath), self.WANT_SCRIPT_NAME)
> if os.path.exists(script_path):
> - proc = subprocess.Popen(
> - [script_path], stdout=subprocess.PIPE,
> - stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> - cwd=path)
> + try:
> + proc = subprocess.Popen(
> + [script_path], stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> + cwd=path)
> + except OSError, e:

Probably should use "except OSError as e" for forward compatibility. (We only
have to support >=Python 2.6, right?)

> + if e.errno == errno.ENOENT:
> + raise ExternalHelperFailed(
> + "%s does not exist" % script_path)
> + raise
> out, ignore = proc.communicate()
> if proc.returncode != 0:
> # TODO: some info on the failure
> @@ -216,6 +224,7 @@
> backends = []
> names = set()
> for path in self.paths:
> + path = os.path.abspath(path)
> if not os.path.isdir(path):
> continue
> fnames = os.listdir(path)

=== modified file 'pkgme/project_info.py'
--- pkgme/project_info.py 2010-11-10 00:44:43 +0000
+++ pkgme/project_info.py 2011-01-16 19:09:53 +0000
> @@ -1,3 +1,4 @@
> +import errno
> import json
> import os
> import subprocess
> @@ -40,10 +41,15 @@
> def get_all(self, keys):
> script_path = os.path.join(self.basepath, self.INFO_SCRIPT_NAME)
> if os.path.exists(script_path):
> - proc = subprocess.Popen(
> - [script_path], stdout=subprocess.PIPE,
> - stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> - cwd=self.cwd)
> + try:
> + proc = subprocess.Popen(
> + [script_path], stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> + cwd=self.cwd)
> + except OSError, e:

except OSError as e:

> + if e.errno == errno.ENOENT:
> + raise ExternalHelperFailed(
> + "%s does not exist" % script_path)
> out, ignore = proc.communicate(json.dumps(keys))
> if proc.returncode != 0:
> raise ExternalHelperFailed(
> @@ -59,10 +65,15 @@
> def _get(self, key):
> script_path = os.path.join(self.basepath, key)
> if os.path.exists(script_path):
> - proc = subprocess.Popen(
> - [script_path], stdout=subprocess.PIPE,
> - stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> - cwd=self.cwd)
> + try:
> + proc = subprocess.Popen(
> + [script_path], stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
> + cwd=self.cwd)
> + except OSError, e:

except OSError as e:

> + if e.errno == errno.ENOENT:
> + raise ExternalHelperFailed(
> + "%s does not exist" % script_path)
> out, ignore = proc.communicate()
> if proc.returncode != 0:
> # TODO: some info on the failure

=== modified file 'pkgme/tests/test_vala_backend.py'
--- pkgme/tests/test_vala_backend.py 2010-12-18 14:40:42 +0000
+++ pkgme/tests/test_vala_backend.py 2011-01-16 19:09:53 +0000
> @@ -31,10 +31,10 @@
> backend_dir = get_backend_dir(__file__, 'vala')
>
>
> -class PythonBackendTests(TestCase, TestWithFixtures):
> +class ValaBackendTests(TestCase, TestWithFixtures):
>
> def setUp(self):
> - super(PythonBackendTests, self).setUp()
> + super(ValaBackendTests, self).setUp()
> self.tempdir = self.useFixture(TempdirFixture())
>
> def write_file_in_tempdir(self, path, content):