Merge lp:~clint-fewbar/pyjuju/no-lsb-release into lp:pyjuju

Proposed by Clint Byrum on 2012-10-23
Status: Merged
Approved by: Jim Baker on 2012-10-24
Approved revision: 599
Merged at revision: 599
Proposed branch: lp:~clint-fewbar/pyjuju/no-lsb-release
Merge into: lp:pyjuju
Diff against target: 30 lines (+10/-8)
1 file modified
juju/charm/tests/test_directory.py (+10/-8)
To merge this branch: bzr merge lp:~clint-fewbar/pyjuju/no-lsb-release
Reviewer Review Type Date Requested Status
Juju Engineering 2012-10-23 Pending
Review via email: mp+130929@code.launchpad.net

Description of the change

Fixes tests relying on /etc/lsb-release

Fixes tests relying on /etc/lsb-release

https://codereview.appspot.com/6750055/

To post a comment you must log in.
Clint Byrum (clint-fewbar) wrote :

Reviewers: mp+130929_code.launchpad.net,

Message:
Please take a look.

Description:
Fixes tests relying on /etc/lsb-release

Fixes tests relying on /etc/lsb-release

https://code.launchpad.net/~clint-fewbar/juju/no-lsb-release/+merge/130929

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6750055/

Affected files:
   A [revision details]
   M juju/charm/tests/test_directory.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: juju/charm/tests/test_directory.py
=== modified file 'juju/charm/tests/test_directory.py'
--- juju/charm/tests/test_directory.py 2012-09-14 15:33:33 +0000
+++ juju/charm/tests/test_directory.py 2012-10-23 03:30:14 +0000
@@ -212,16 +212,18 @@

      def test_internal_symlink(self):
          charm_path = self.copy_charm()
- os.symlink("/etc/lsb-release", os.path.join(charm_path, "foobar"))
+ with tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
+ os.symlink(external_file.name,
os.path.join(charm_path, "foobar"))

- directory = CharmDirectory(charm_path)
- e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
- self.assertIn("foobar' Absolute links are invalid", str(e))
+ directory = CharmDirectory(charm_path)
+ e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
+ self.assertIn("foobar' Absolute links are invalid", str(e))

      def test_extract_symlink(self):
          charm_path = self.copy_charm()
- os.symlink("/etc/lsb-release", os.path.join(charm_path, "foobar"))
+ with tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
+ os.symlink(external_file.name,
os.path.join(charm_path, "foobar"))

- directory = CharmDirectory(charm_path)
- e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
- self.assertIn("foobar' Absolute links are invalid", str(e))
+ directory = CharmDirectory(charm_path)
+ e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
+ self.assertIn("foobar' Absolute links are invalid", str(e))

Jim Baker (jimbaker) wrote :

+1, LGTM, with the following coding convention minor addressed.

https://codereview.appspot.com/6750055/diff/1/juju/charm/tests/test_directory.py
File juju/charm/tests/test_directory.py (right):

https://codereview.appspot.com/6750055/diff/1/juju/charm/tests/test_directory.py#newcode215
juju/charm/tests/test_directory.py:215: with
tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
Use the makeFile method on our TestCase; it constructs a temporary file
as above, but also ensures it is deleted at the end of the test. This
method is used throughout our tests, so this will help with following
current coding conventions.

external_file = self.makeFile()

https://codereview.appspot.com/6750055/diff/1/juju/charm/tests/test_directory.py#newcode224
juju/charm/tests/test_directory.py:224: with
tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
Ditto.

https://codereview.appspot.com/6750055/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/charm/tests/test_directory.py'
2--- juju/charm/tests/test_directory.py 2012-09-14 15:33:33 +0000
3+++ juju/charm/tests/test_directory.py 2012-10-23 03:32:18 +0000
4@@ -212,16 +212,18 @@
5
6 def test_internal_symlink(self):
7 charm_path = self.copy_charm()
8- os.symlink("/etc/lsb-release", os.path.join(charm_path, "foobar"))
9+ with tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
10+ os.symlink(external_file.name, os.path.join(charm_path, "foobar"))
11
12- directory = CharmDirectory(charm_path)
13- e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
14- self.assertIn("foobar' Absolute links are invalid", str(e))
15+ directory = CharmDirectory(charm_path)
16+ e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
17+ self.assertIn("foobar' Absolute links are invalid", str(e))
18
19 def test_extract_symlink(self):
20 charm_path = self.copy_charm()
21- os.symlink("/etc/lsb-release", os.path.join(charm_path, "foobar"))
22+ with tempfile.NamedTemporaryFile(dir='/tmp') as external_file:
23+ os.symlink(external_file.name, os.path.join(charm_path, "foobar"))
24
25- directory = CharmDirectory(charm_path)
26- e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
27- self.assertIn("foobar' Absolute links are invalid", str(e))
28+ directory = CharmDirectory(charm_path)
29+ e = self.assertRaises(InvalidCharmFile, directory.as_bundle)
30+ self.assertIn("foobar' Absolute links are invalid", str(e))

Subscribers

People subscribed via source and target branches

to status/vote changes: