Merge lp:~matsubara/tarmac/better-debug-output into lp:~launchpad/tarmac/lp-tarmac

[Diogo Matsubara]

[r=gary] Changes the bundle lander plugin to keep a persistent log file on disk.

This changes the bundle lander plugin to keep a persistent log file on disk. After an initial review from Gary, I added some more tests showing the the logs are correctly moved and replaced.

[Gary Poster (gary)]

We could have yet more test coverage, but I'm OK with this for now

Thank you

Gary

review: Approve

=== modified file 'tarmac/plugins/bundlecommand.py'
--- tarmac/plugins/bundlecommand.py	2010-12-17 14:19:01 +0000
+++ tarmac/plugins/bundlecommand.py	2011-01-05 17:24:57 +0000
@@ -153,7 +153,18 @@
         self.logger.debug('Running test command: %s' % self.verify_command)
         cwd = os.getcwd()
         os.chdir(target.tree.abspath(''))
-        output = tempfile.TemporaryFile()
+        # Output must be logged and kept in the filesystem to help debugging.
+        current_log = os.path.join(
+            target.config.debug_log_location, 'current.log')
+        previous_log = os.path.join(
+            target.config.debug_log_location, 'previous.log')
+        if os.path.exists(current_log):
+            # Move it out of the way. Trashes previous.log.
+            os.rename(current_log, previous_log)
+        # Create a new empty file to log the output.
+        open(current_log, 'w').close()
+        # File must be opened for read and write.
+        output = open(current_log, 'rw')
         try:
             proc = subprocess.Popen(
                 self.verify_command,

=== modified file 'tarmac/plugins/tests/test_bundlecommand.py'
--- tarmac/plugins/tests/test_bundlecommand.py	2010-12-17 12:58:39 +0000
+++ tarmac/plugins/tests/test_bundlecommand.py	2011-01-05 17:24:57 +0000
@@ -223,7 +223,8 @@
     def test_run(self):
         """Test that the plug-in run without errors."""
         target = Thing(config=Thing(
-            bundle_verify_command="/bin/true"),
+            bundle_verify_command="/bin/true",
+            debug_log_location=self.TEST_ROOT),
             tree=Thing(abspath=os.path.abspath))
         self.plugin.run(
             command=self.command, target=target, restore_revno=123,
@@ -244,6 +245,7 @@
         target = self.branch1
         # Include the config to run the bundle verify command.
         setattr(target.config, 'bundle_verify_command', '/bin/false')
+        setattr(target.config, 'debug_log_location', self.TEST_ROOT)
         # Set a fake method to target, so we can make sure it's called when
         # the plugin fails to run.
         def fake_uncommit_and_revert(restore_revno):
@@ -279,6 +281,34 @@
         # Check the patched target.uncommit_and_revert() was run.
         self.assertEqual(target.fake_revno, 123)
 
+    def test_debug_log(self):
+        """Test logs are kept after plugin run."""
+        current_log = os.path.join(self.TEST_ROOT, 'current.log')
+        previous_log = os.path.join(self.TEST_ROOT, 'previous.log')
+        self.assertFalse(os.path.exists(current_log))
+        # Create a fake log file in current.log, so it can be moved to
+        # previous.log
+        log_file = open(current_log, 'w')
+        log_file.write('Foobar')
+        log_file.close()
+        # Set up config for plugin.
+        target = Thing(config=Thing(
+            bundle_verify_command='/bin/true',
+            debug_log_location=self.TEST_ROOT),
+            tree=Thing(abspath=os.path.abspath))
+        # Run.
+        self.plugin.run(
+            command=self.command, target=target, restore_revno=123,
+            collected_proposals=self.collected_proposals)
+        # Check we have current.log in place with no content since /bin/true
+        # doesn't actually gives any output.
+        self.assertTrue(os.path.exists(current_log))
+        self.assertEqual('', open(current_log, 'r').read())
+        # Check fake log was moved correctly to previous.log and the content
+        # match.
+        self.assertTrue(os.path.exists(previous_log))
+        self.assertEqual('Foobar', open(previous_log, 'r').read())
+
     def test_send_report_email(self):
         """Test send_report_email()."""
         formatter = self.make_formatter()