Merge lp:~veebers/autopilot/fix_1203716_patch_environment into lp:autopilot

[Christopher Lee]

Fixes bug 1203716 where patch_environment didn't cleanup after itself.

Fixes bug 1203716 where patch_environment didn't cleanup after itself.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:281
http://jenkins.qa.ubuntu.com/job/autopilot-ci/167/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/95
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/95

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/167/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:283
http://jenkins.qa.ubuntu.com/job/autopilot-ci/172/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/100
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/100

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/172/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:284
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~veebers/autopilot/fix_1203716_patch_environment/+merge/176578/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/autopilot-ci/174/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/102
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/102

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/174/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:284
http://jenkins.qa.ubuntu.com/job/autopilot-ci/175/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-amd64-ci/103
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-saucy-armhf-ci/103

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-ci/175/rebuild

review: Approve (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

LGTM

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'autopilot/testcase.py'
--- autopilot/testcase.py	2013-07-22 04:02:05 +0000
+++ autopilot/testcase.py	2013-07-24 02:56:31 +0000
@@ -321,10 +321,26 @@
 
         """
         if key in os.environ:
+            def _undo_patch(key, old_value):
+                logger.info(
+                    "Resetting environment variable '%s' to '%s'",
+                    key,
+                    old_value
+                )
+                os.environ[key] = old_value
             old_value = os.environ[key]
-            self.addCleanup(os.putenv, key, old_value)
+            self.addCleanup(_undo_patch, key, old_value)
         else:
-            self.addCleanup(os.unsetenv, key)
+            def _remove_patch(key):
+                try:
+                    del os.environ[key]
+                except KeyError:
+                    logger.warning(
+                        "Attempted to delete environment key '%s' that doesn't"
+                        "exist in the environment",
+                        key
+                    )
+            self.addCleanup(_remove_patch, key)
         os.environ[key] = value
 
     def assertVisibleWindowStack(self, stack_start):

=== modified file 'autopilot/tests/functional/test_autopilot_functional.py'
--- autopilot/tests/functional/test_autopilot_functional.py	2013-07-14 10:01:01 +0000
+++ autopilot/tests/functional/test_autopilot_functional.py	2013-07-24 02:56:31 +0000
@@ -869,6 +869,50 @@
         self.assertThat(output, Not(Contains('Running tests in random order')))
 
 
+class AutopilotPatchEnvironmentTests(AutopilotTestCase):
+
+    def test_patch_environment_new_patch_is_unset_to_none(self):
+        """patch_environment must unset the environment variable if previously
+        was unset.
+
+        """
+
+        class PatchEnvironmentSubTests(AutopilotTestCase):
+
+            def test_patch_env_sets_var(self):
+                """Setting the environment variable must make it available."""
+                self.patch_environment("APABC321", "Foo")
+                self.assertThat(os.getenv("APABC321"), Equals("Foo"))
+
+        self.assertThat(os.getenv('APABC321'), Equals(None))
+
+        result = PatchEnvironmentSubTests("test_patch_env_sets_var").run()
+
+        self.assertThat(result.wasSuccessful(), Equals(True))
+        self.assertThat(os.getenv('APABC321'), Equals(None))
+
+    def test_patch_environment_existing_patch_is_reset(self):
+        """patch_environment must reset the environment back to it's previous
+        value.
+
+        """
+
+        class PatchEnvironmentSubTests(AutopilotTestCase):
+
+            def test_patch_env_sets_var(self):
+                """Setting the environment variable must make it available."""
+                self.patch_environment("APABC987", "InnerTest")
+                self.assertThat(os.getenv("APABC987"), Equals("InnerTest"))
+
+        self.patch_environment('APABC987', "OuterTest")
+        self.assertThat(os.getenv('APABC987'), Equals("OuterTest"))
+
+        result = PatchEnvironmentSubTests("test_patch_env_sets_var").run()
+
+        self.assertThat(result.wasSuccessful(), Equals(True))
+        self.assertThat(os.getenv('APABC987'), Equals("OuterTest"))
+
+
 class AutopilotVerboseFunctionalTests(AutopilotFunctionalTestsBase):
 
     """Scenarioed functional tests for autopilot's verbose logging."""