pkgme

Merge lp:~james-w/pkgme/further-test-fixes into lp:pkgme

  1. further-test-fixes
  2. Merge into trunk
Proposed by James Westby
Status: Merged
Approved by: Barry Warsaw
Approved revision: 44
Merged at revision: 47
Proposed branch: lp:~james-w/pkgme/further-test-fixes
Merge into: lp:pkgme
Prerequisite: lp:~james-w/pkgme/add-docs
Diff against target: 116 lines (+36/-16)
3 files modified
pkgme/backend.py (+15/-6)
pkgme/project_info.py (+19/-8)
pkgme/tests/test_vala_backend.py (+2/-2)
To merge this branch: bzr merge lp:~james-w/pkgme/further-test-fixes
Reviewer Review Type Date Requested Status
Barry Warsaw (community) Approve
Review via email: mp+46419@code.launchpad.net

Description of the change

Hi,

Here are some further improvements that prevented me from running
some of the tests, or helped me diagnose those problems.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Barry Warsaw (barry) wrote :
Download full text (5.1 KiB)

 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.PI...

Read more...

review: Approve
Revision history for this message
James Westby (james-w) wrote :

On Mon, 24 Jan 2011 19:28:31 -0000, Barry Warsaw <email address hidden> wrote:
> > + 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?)

I guess so. Someone is bound to request 2.3 support some day, but I'm
fine with 2.6 for now.

Thanks,

James

lp:~james-w/pkgme/further-test-fixes updated
45. By James Westby

Tweaks from review. Thanks Barry.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'pkgme/backend.py'
2--- pkgme/backend.py 2010-12-18 14:55:00 +0000
3+++ pkgme/backend.py 2011-02-05 21:09:09 +0000
4@@ -1,3 +1,4 @@
5+import errno
6 import os
7 import sys
8 import subprocess
9@@ -16,7 +17,7 @@
10
11
12 def get_backend_dir(underunder_file, backend):
13- backend_dir = os.path.normpath(
14+ backend_dir = os.path.abspath(
15 os.path.join(
16 os.path.dirname(underunder_file), os.pardir,
17 'backends', backend))
18@@ -114,12 +115,19 @@
19 self.basepath = basepath
20
21 def want(self, path):
22- script_path = os.path.join(self.basepath, self.WANT_SCRIPT_NAME)
23+ script_path = os.path.join(
24+ os.path.abspath(self.basepath), self.WANT_SCRIPT_NAME)
25 if os.path.exists(script_path):
26- proc = subprocess.Popen(
27- [script_path], stdout=subprocess.PIPE,
28- stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
29- cwd=path)
30+ try:
31+ proc = subprocess.Popen(
32+ [script_path], stdout=subprocess.PIPE,
33+ stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
34+ cwd=path)
35+ except OSError as e:
36+ if e.errno == errno.ENOENT:
37+ raise ExternalHelperFailed(
38+ "%s does not exist" % script_path)
39+ raise
40 out, ignore = proc.communicate()
41 if proc.returncode != 0:
42 # TODO: some info on the failure
43@@ -216,6 +224,7 @@
44 backends = []
45 names = set()
46 for path in self.paths:
47+ path = os.path.abspath(path)
48 if not os.path.isdir(path):
49 continue
50 fnames = os.listdir(path)
51
52=== modified file 'pkgme/project_info.py'
53--- pkgme/project_info.py 2010-11-10 00:44:43 +0000
54+++ pkgme/project_info.py 2011-02-05 21:09:09 +0000
55@@ -1,3 +1,4 @@
56+import errno
57 import json
58 import os
59 import subprocess
60@@ -40,10 +41,15 @@
61 def get_all(self, keys):
62 script_path = os.path.join(self.basepath, self.INFO_SCRIPT_NAME)
63 if os.path.exists(script_path):
64- proc = subprocess.Popen(
65- [script_path], stdout=subprocess.PIPE,
66- stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
67- cwd=self.cwd)
68+ try:
69+ proc = subprocess.Popen(
70+ [script_path], stdout=subprocess.PIPE,
71+ stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
72+ cwd=self.cwd)
73+ except OSError as e:
74+ if e.errno == errno.ENOENT:
75+ raise ExternalHelperFailed(
76+ "%s does not exist" % script_path)
77 out, ignore = proc.communicate(json.dumps(keys))
78 if proc.returncode != 0:
79 raise ExternalHelperFailed(
80@@ -59,10 +65,15 @@
81 def _get(self, key):
82 script_path = os.path.join(self.basepath, key)
83 if os.path.exists(script_path):
84- proc = subprocess.Popen(
85- [script_path], stdout=subprocess.PIPE,
86- stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
87- cwd=self.cwd)
88+ try:
89+ proc = subprocess.Popen(
90+ [script_path], stdout=subprocess.PIPE,
91+ stderr=subprocess.STDOUT, stdin=subprocess.PIPE,
92+ cwd=self.cwd)
93+ except OSError as e:
94+ if e.errno == errno.ENOENT:
95+ raise ExternalHelperFailed(
96+ "%s does not exist" % script_path)
97 out, ignore = proc.communicate()
98 if proc.returncode != 0:
99 # TODO: some info on the failure
100
101=== modified file 'pkgme/tests/test_vala_backend.py'
102--- pkgme/tests/test_vala_backend.py 2010-12-18 14:40:42 +0000
103+++ pkgme/tests/test_vala_backend.py 2011-02-05 21:09:09 +0000
104@@ -31,10 +31,10 @@
105 backend_dir = get_backend_dir(__file__, 'vala')
106
107
108-class PythonBackendTests(TestCase, TestWithFixtures):
109+class ValaBackendTests(TestCase, TestWithFixtures):
110
111 def setUp(self):
112- super(PythonBackendTests, self).setUp()
113+ super(ValaBackendTests, self).setUp()
114 self.tempdir = self.useFixture(TempdirFixture())
115
116 def write_file_in_tempdir(self, path, content):

Subscribers

People subscribed via source and target branches

to all changes: