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

[Christopher Lee]

Added an extra check so that a 'failing' test wasn't recorded if it was in fact a skipping test.

Added an extra check so that a 'failing' test wasn't recorded if it was in fact a skipping test.

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Chris Gagnon (chris.gagnon)]

Can you add a test for the new if statement?

review: Needs Fixing

[lp:~veebers/autopilot/fix_too_many_vids_1189655]

242. By Christopher Lee on 2013-06-12

Added missing test for video recording and skipped tests

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[Chris Gagnon (chris.gagnon)]

Looks good, thanks for writing the test

review: Approve

=== modified file 'autopilot/globals.py'
--- autopilot/globals.py	2013-05-30 12:00:24 +0000
+++ autopilot/globals.py	2013-06-12 00:43:25 +0000
@@ -163,7 +163,10 @@
 
     def _on_test_failed(self, ex_info):
         """Called when a test fails."""
-        self._test_passed = False
+        from unittest.case import SkipTest
+        failure_class_type = ex_info[0]
+        if failure_class_type is not SkipTest:
+            self._test_passed = False
 
 
 _video_logger = _VideoLogger()

=== modified file 'autopilot/tests/functional/test_autopilot_functional.py'
--- autopilot/tests/functional/test_autopilot_functional.py	2013-05-30 21:35:13 +0000
+++ autopilot/tests/functional/test_autopilot_functional.py	2013-06-12 00:43:25 +0000
@@ -466,7 +466,7 @@
         if should_delete:
             self.addCleanup(remove_if_exists, ap_dir)
         else:
-            self.addCleanup(remove_if_exists, 
+            self.addCleanup(remove_if_exists,
                             '%s/tests.test_simple.SimpleTest.test_simple.ogv' % (ap_dir))
 
         code, output, error = self.run_autopilot(["run", "-r", "-rd", video_dir, "tests"])
@@ -526,6 +526,32 @@
         self.assertFalse(os.path.exists(
             '/tmp/autopilot/tests.test_simple.SimpleTest.test_simple.ogv'))
 
+    def test_no_videos_saved_for_skipped_test(self):
+        """Videos must not be saved if the test has been skipped (not
+        failed).
+
+        """
+        self.create_test_file("test_simple.py", dedent("""\
+
+            from autopilot.testcase import AutopilotTestCase
+            from time import sleep
+
+            class SimpleTest(AutopilotTestCase):
+
+                def test_simple(self):
+                    sleep(1)
+                    self.skip("Skipping Test")
+            """
+            ))
+
+        video_file_path = '/tmp/autopilot/tests.test_simple.SimpleTest.test_simple.ogv'
+        self.addCleanup(remove_if_exists, video_file_path)
+
+        code, output, error = self.run_autopilot(["run", "-r", "tests"])
+
+        self.assertThat(code, Equals(0))
+        self.assertThat(os.path.exists(video_file_path), Equals(False))
+
     def test_runs_with_import_errors_fail(self):
         """Import errors inside a test must be considered a test failure."""
         self.create_test_file('test_simple.py', dedent("""\