launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/librarian-log-bug-828151 into lp:launchpad with lp:~allenap/launchpad/native-sync-real-names-in-emails-bug-827526 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #828151 in Launchpad itself: "The librarian-log test detail contains the whole librarian log, not just the log from the currently running test"
  https://bugs.launchpad.net/launchpad/+bug/828151

For more details, see:
https://code.launchpad.net/~allenap/launchpad/librarian-log-bug-828151/+merge/72097

This adds a truncateLog() method to TacTestSetup - upon with
LibrarianServerFixture is based - and gets _check_and_reset() in
LibrarianLayer to call it.
-- 
https://code.launchpad.net/~allenap/launchpad/librarian-log-bug-828151/+merge/72097
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/librarian-log-bug-828151 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/daemons/tachandler.py'
--- lib/canonical/launchpad/daemons/tachandler.py	2011-08-18 10:13:17 +0000
+++ lib/canonical/launchpad/daemons/tachandler.py	2011-08-19 14:28:14 +0000
@@ -12,6 +12,7 @@
 
 
 import os
+import os.path
 import subprocess
 import sys
 import time
@@ -151,6 +152,24 @@
             return
         os.kill(pid, sig)
 
+    def truncateLog(self):
+        """Truncate the log file.
+
+        Leaves everything up to and including the `LOG_MAGIC` marker in
+        place. If the `LOG_MAGIC` marker is not found the log is truncated to
+        0 bytes.
+        """
+        if os.path.exists(self.logfile):
+            with open(self.logfile, "r+b") as logfile:
+                position = 0
+                for line in logfile:
+                    position += len(line)
+                    if readyservice.LOG_MAGIC in line:
+                        logfile.truncate(position)
+                        break
+                else:
+                    logfile.truncate(0)
+
     def setUpRoot(self):
         """Override this.
 

=== modified file 'lib/canonical/launchpad/daemons/tests/test_tachandler.py'
--- lib/canonical/launchpad/daemons/tests/test_tachandler.py	2011-08-18 11:14:54 +0000
+++ lib/canonical/launchpad/daemons/tests/test_tachandler.py	2011-08-19 14:28:14 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+import os.path
 from os.path import (
     dirname,
     exists,
@@ -21,6 +22,7 @@
     Not,
     )
 
+from canonical.launchpad.daemons.readyservice import LOG_MAGIC
 from canonical.launchpad.daemons.tachandler import (
     TacException,
     TacTestSetup,
@@ -132,3 +134,40 @@
         # One deprecation warning is emitted.
         self.assertEqual(1, len(warnings_log))
         self.assertIs(DeprecationWarning, warnings_log[0].category)
+
+    def test_truncateLog(self):
+        """
+        truncateLog truncates the log, if it exists, leaving the record of the
+        service start in place.
+        """
+        tempdir = self.useFixture(TempDir()).path
+        fixture = SimpleTac("okay.tac", tempdir)
+
+        # Truncating the log is a no-op if the log does not exist.
+        fixture.truncateLog()
+        self.assertFalse(os.path.exists(fixture.logfile))
+
+        # Put something in the log file.
+        with open(fixture.logfile, "wb") as logfile:
+            logfile.write("Hello\n")
+
+        # Truncating the log does not remove the log file.
+        fixture.truncateLog()
+        self.assertTrue(os.path.exists(fixture.logfile))
+        with open(fixture.logfile, "rb") as logfile:
+            self.assertEqual("", logfile.read())
+
+        # Put something in the log again, along with LOG_MAGIC.
+        with open(fixture.logfile, "wb") as logfile:
+            logfile.write("One\n")
+            logfile.write("Two\n")
+            logfile.write("Three, %s\n" % LOG_MAGIC)
+            logfile.write("Four\n")
+
+        # Truncating the log leaves everything up to and including the line
+        # containing LOG_MAGIC.
+        fixture.truncateLog()
+        with open(fixture.logfile, "rb") as logfile:
+            self.assertEqual(
+                "One\nTwo\nThree, %s\n" % LOG_MAGIC,
+                logfile.read())

=== modified file 'lib/canonical/librarian/testing/server.py'
--- lib/canonical/librarian/testing/server.py	2011-06-21 16:47:51 +0000
+++ lib/canonical/librarian/testing/server.py	2011-08-19 14:28:14 +0000
@@ -228,6 +228,11 @@
         """Get a list with the contents of the librarian log in it."""
         return open(self.logfile, 'rb').readlines()
 
+    def reset(self):
+        """Reset the librarian to a consistent initial state."""
+        self.clear()
+        self.truncateLog()
+
 
 def fillLibrarianFile(fileid, content='Fake Content'):
     """Write contents in disk for a librarian sampledata."""

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2011-08-12 14:39:51 +0000
+++ lib/canonical/testing/layers.py	2011-08-19 14:28:14 +0000
@@ -913,9 +913,7 @@
     @classmethod
     @profiled
     def _check_and_reset(cls):
-        """Raise an exception if the Librarian has been killed.
-        Reset the storage unless this has been disabled.
-        """
+        """Raise an exception if the Librarian has been killed, else reset."""
         try:
             f = urlopen(config.librarian.download_url)
             f.read()
@@ -926,7 +924,8 @@
                     "LibrarianLayer.reveal() where possible, and ensure "
                     "the Librarian is restarted if it absolutely must be "
                     "shutdown: " + str(e))
-        cls.librarian_fixture.clear()
+        else:
+            cls.librarian_fixture.reset()
 
     @classmethod
     @profiled