Merge lp:~jml/launchpad/dont-print-in-tests into lp:launchpad

[Jonathan Lange]

[r=jelmer][ui=none][no-qa] Fix some tests to not print stuff

This branch fixes a bunch of tests to not print stuff to stdout / stderr during the test run.

It doesn't fix all of them, because bug #694152 makes it really hard to do so and because I have absolutely no idea what's going on with the 'lazr.smtptest' log handler.

[Jelmer Vernooij (jelmer)]

As discussed on IRC, r=me for this change without the call to patch().

14:59 < jelmer> jml: How hard would it be to avoid the patch() call in
                test_translationtemplatesbuildmanager.py ?
15:00 < jml> jelmer: good question.
15:00 < jml> I think I'd need to change some stuff to not do "print "
15:03 < jml> jelmer: I think I'd have to change lib/canonical/buildd/debian.py
15:03 < jml> jelmer: I really didn't want to do that
15:05 < jml> brb
15:10 < jml> back
15:55 < jelmer> re
15:58 < jelmer> jml: I think that should be doable.
15:59 < jelmer> jml: My worry is that "eating" all of the stdout output might hide some
                useful messages printed out by other bits of the code.
15:59 < jml> jelmer: ok. I'll give it a go.
16:00 < jelmer> jml: If you like, I'd be happy to +1 the changes without the patch()
                call for now?

review: Approve (code)

=== modified file 'lib/canonical/buildd/pottery/generate_translation_templates.py'
--- lib/canonical/buildd/pottery/generate_translation_templates.py	2010-05-19 10:27:26 +0000
+++ lib/canonical/buildd/pottery/generate_translation_templates.py	2010-12-30 15:05:26 +0000
@@ -19,7 +19,7 @@
 class GenerateTranslationTemplates:
     """Script to generate translation templates from a branch."""
 
-    def __init__(self, branch_spec, result_name, work_dir):
+    def __init__(self, branch_spec, result_name, work_dir, log_file=None):
         """Prepare to generate templates for a branch.
 
         :param branch_spec: Either a branch URL or the path of a local
@@ -29,17 +29,21 @@
         :param result_name: The name of the result tarball. Should end in
             .tar.gz.
         :param work_dir: The directory to work in. Must exist.
+        :param log_file: File-like object to log to. If None, defaults to
+            stderr.
         """
         self.work_dir = work_dir
         self.branch_spec = branch_spec
         self.result_name = result_name
-        self.logger = self._setupLogger()
+        self.logger = self._setupLogger(log_file)
 
-    def _setupLogger(self):
+    def _setupLogger(self, log_file):
         """Sets up and returns a logger."""
+        if log_file is None:
+            log_file = sys.stderr
         logger = logging.getLogger("generate-templates")
         logger.setLevel(logging.DEBUG)
-        ch = logging.StreamHandler()
+        ch = logging.StreamHandler(log_file)
         ch.setLevel(logging.DEBUG)
         logger.addHandler(ch)
         return logger

=== modified file 'lib/canonical/buildd/tests/test_generate_translation_templates.py'
--- lib/canonical/buildd/tests/test_generate_translation_templates.py	2010-04-28 05:40:02 +0000
+++ lib/canonical/buildd/tests/test_generate_translation_templates.py	2010-12-30 15:05:26 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import os
+from StringIO import StringIO
 from unittest import TestLoader
 import tarfile
 
@@ -28,7 +29,8 @@
         branch_url = 'lp://~my/translation/branch'
 
         generator = GenerateTranslationTemplates(
-            branch_url, self.result_name, self.makeTemporaryDirectory())
+            branch_url, self.result_name, self.makeTemporaryDirectory(),
+            log_file=StringIO())
         generator._checkout = FakeMethod()
         generator._getBranch()
 
@@ -41,7 +43,8 @@
         branch_dir = '/home/me/branch'
 
         generator = GenerateTranslationTemplates(
-            branch_dir, self.result_name, self.makeTemporaryDirectory())
+            branch_dir, self.result_name, self.makeTemporaryDirectory(),
+            log_file=StringIO())
         generator._checkout = FakeMethod()
         generator._getBranch()
 
@@ -50,7 +53,7 @@
 
     def _createBranch(self, content_map=None):
         """Create a working branch.
-        
+
         :param content_map: optional dict mapping file names to file contents.
             Each of these files with their contents will be written to the
             branch.
@@ -77,7 +80,7 @@
 
         generator = GenerateTranslationTemplates(
             branch.getInternalBzrUrl(), self.result_name,
-            self.makeTemporaryDirectory())
+            self.makeTemporaryDirectory(), log_file=StringIO())
         generator.branch_dir = self.makeTemporaryDirectory()
         generator._getBranch()
 
@@ -96,7 +99,7 @@
         tar.close()
 
         generator = GenerateTranslationTemplates(
-            branchdir, self.result_name, workdir)
+            branchdir, self.result_name, workdir, log_file=StringIO())
         generator._getBranch()
         generator._makeTarball(potnames)
         tar = tarfile.open(os.path.join(workdir, self.result_name), 'r|*')

=== modified file 'lib/canonical/librarian/smoketest.py'
--- lib/canonical/librarian/smoketest.py	2010-12-07 15:13:27 +0000
+++ lib/canonical/librarian/smoketest.py	2010-12-30 15:05:26 +0000
@@ -8,6 +8,7 @@
 
 from cStringIO import StringIO
 import datetime
+import sys
 import urllib
 
 from zope.component import getUtility
@@ -46,19 +47,21 @@
     return data
 
 
-def do_smoketest(restricted_client, regular_client):
-    print 'adding a private file to the librarian...'
+def do_smoketest(restricted_client, regular_client, output=None):
+    if output is None:
+        output = sys.stdout
+    output.write('adding a private file to the librarian...\n')
     private_url = store_file(restricted_client)
-    print 'retrieving private file from', private_url
+    output.write('retrieving private file from %s\n' % (private_url,))
     if read_file(private_url) != FILE_DATA:
-        print 'ERROR: data fetched does not match data written'
+        output.write('ERROR: data fetched does not match data written\n')
         return 1
 
-    print 'adding a public file to the librarian...'
+    output.write('adding a public file to the librarian...\n')
     public_url = store_file(regular_client)
-    print 'retrieving public file from', public_url
+    output.write('retrieving public file from %s\n' % (public_url,))
     if read_file(public_url) != FILE_DATA:
-        print 'ERROR: data fetched does not match data written'
+        output.write('ERROR: data fetched does not match data written\n')
         return 1
 
     return 0

=== modified file 'lib/canonical/librarian/tests/test_smoketest.py'
--- lib/canonical/librarian/tests/test_smoketest.py	2010-12-07 15:13:27 +0000
+++ lib/canonical/librarian/tests/test_smoketest.py	2010-12-30 15:05:26 +0000
@@ -79,7 +79,8 @@
         # exit code to signal success).
         with fake_urllib(GoodUrllib()):
             self.assertEquals(
-                do_smoketest(self.fake_librarian, self.fake_librarian),
+                do_smoketest(self.fake_librarian, self.fake_librarian,
+                             output=StringIO()),
                 0)
 
     def test_bad_data(self):
@@ -87,7 +88,8 @@
         # (which will be used as the processes exit code to signal an error).
         with fake_urllib(BadUrllib()):
             self.assertEquals(
-                do_smoketest(self.fake_librarian, self.fake_librarian),
+                do_smoketest(self.fake_librarian, self.fake_librarian,
+                             output=StringIO()),
                 1)
 
     def test_exception(self):
@@ -96,7 +98,8 @@
         # code to signal an error).
         with fake_urllib(ErrorUrllib()):
             self.assertEquals(
-                do_smoketest(self.fake_librarian, self.fake_librarian),
+                do_smoketest(self.fake_librarian, self.fake_librarian,
+                             output=StringIO()),
                 1)
 
     def test_explosive_errors(self):
@@ -106,4 +109,5 @@
             with fake_urllib(ExplosiveUrllib(exception)):
                 self.assertRaises(
                     exception,
-                    do_smoketest, self.fake_librarian, self.fake_librarian)
+                    do_smoketest, self.fake_librarian, self.fake_librarian,
+                    output=StringIO())

=== modified file 'lib/canonical/testing/tests/test_parallel.py'
--- lib/canonical/testing/tests/test_parallel.py	2010-10-26 15:47:24 +0000
+++ lib/canonical/testing/tests/test_parallel.py	2010-12-30 15:05:26 +0000
@@ -22,7 +22,6 @@
     find_load_list,
     find_tests,
     ListTestCase,
-    main,
     prepare_argv,
     )
 
@@ -103,13 +102,13 @@
             self.assertEqual(
                 ['bin/test', '-vt', 'filter', '--list-tests', '--subunit'],
                 args['args'])
-            return {'stdin': StringIO(), 'stdout': StringIO(u"""
+            return {'stdin': StringIO(), 'stdout': StringIO(u"""\
 test: quux
 successful: quux
 test: glom
 successful: glom
 """)}
-        popen = self.useFixture(PopenFixture(inject_testlist))
+        self.useFixture(PopenFixture(inject_testlist))
         self.assertEqual(
             ['quux', 'glom'],
             find_tests(['bin/test', '-vt', 'filter', '--parallel']))

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2010-12-20 13:44:10 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2010-12-30 15:05:26 +0000
@@ -547,7 +547,6 @@
             current_request=True)
         contents = view.render()
         help_link = find_tag_by_id(contents, 'getting-started-help')
-        print help_link
         self.assertIs(None, help_link)
 
 

=== modified file 'lib/lp/bugs/doc/checkwatches-cli-switches.txt'
--- lib/lp/bugs/doc/checkwatches-cli-switches.txt	2010-12-22 20:55:25 +0000
+++ lib/lp/bugs/doc/checkwatches-cli-switches.txt	2010-12-30 15:05:26 +0000
@@ -52,8 +52,10 @@
     ...     def __init__(self, name, dbuser=None, test_args=None):
     ...         super(TestCheckWatchesCronScript, self).__init__(
     ...             name, dbuser, test_args)
+    ...         self.txn = ZopelessTransactionManager
+    ...
+    ...     def handle_options(self):
     ...         self.logger = FakeLogger()
-    ...         self.txn = ZopelessTransactionManager
 
     >>> def run_cronscript_with_args(args):
     ...     # It may seem a bit weird to do ths rather than letting the
@@ -70,6 +72,7 @@
     >>> run_cronscript_with_args([
     ...     '--bug-tracker=mozilla.org', '--bug-tracker=debbugs', '-v',
     ...     '--batch-size=10'])
+    DEBUG Enabled by DEFAULT section
     DEBUG Using a global batch size of 10
     DEBUG No watches to update on https://bugzilla.mozilla.org/
     DEBUG No watches to update on http://bugs.debian.org
@@ -101,6 +104,7 @@
     >>> LaunchpadZopelessLayer.switchDbUser(config.checkwatches.dbuser)
 
     >>> run_cronscript_with_args(['-vvt', 'savannah', '--reset'])
+    DEBUG Enabled by DEFAULT section
     INFO Resetting 5 bug watches for bug tracker 'savannah'
     INFO Updating 5 watches on bug tracker 'savannah'
     WARNING ExternalBugtracker for BugTrackerType 'SAVANE' is not known.

=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2010-12-20 03:21:03 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2010-12-30 15:05:26 +0000
@@ -27,6 +27,7 @@
     NotBranchError,
     )
 from bzrlib.tests import TestCaseWithTransport
+from bzrlib import trace
 from bzrlib.transport import get_transport
 from bzrlib.upgrade import upgrade
 from bzrlib.urlutils import (
@@ -132,7 +133,10 @@
     """Tests for `BazaarBranchStore`."""
 
     def setUp(self):
-        super(TestBazaarBranchStore, self).setUp()
+        WorkerTest.setUp(self)
+        # XXX: JonathanLange 2010-12-24 bug=694140: Avoid spurious "No
+        # handlers for logger 'bzr'" messages.
+        trace._bzr_logger = logging.getLogger('bzr')
         self.temp_dir = self.makeTemporaryDirectory()
         self.arbitrary_branch_id = 10
 

=== modified file 'lib/lp/soyuz/doc/queuebuilder.txt'
--- lib/lp/soyuz/doc/queuebuilder.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/doc/queuebuilder.txt	2010-12-30 15:05:26 +0000
@@ -30,8 +30,8 @@
 user designed for this kind of access is 'fiera', see
 test_system_documentation.py for more information.
 
-    >>> import logging
-    >>> logger = logging.getLogger()
+    >>> from lp.services.log.logger import DevNullLogger
+    >>> logger = DevNullLogger()
 
 Now that we have satisfied all of the needs for QueueBuilder, let's
 instantiate it.

=== modified file 'lib/lp/soyuz/scripts/buildd.py'
--- lib/lp/soyuz/scripts/buildd.py	2010-11-02 21:44:42 +0000
+++ lib/lp/soyuz/scripts/buildd.py	2010-12-30 15:05:26 +0000
@@ -8,7 +8,6 @@
 __all__ = [
     'QueueBuilder',
     'RetryDepwait',
-    'SlaveScanner',
     ]
 
 from zope.component import getUtility
@@ -18,7 +17,6 @@
 from lp.archivepublisher.debversion import Version
 from lp.archivepublisher.utils import process_in_batches
 from lp.buildmaster.enums import BuildStatus
-from lp.buildmaster.interfaces.builder import IBuilderSet
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.scripts.base import (

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-12-17 23:59:00 +0000
+++ lib/lp/testing/__init__.py	2010-12-30 15:05:26 +0000
@@ -57,6 +57,7 @@
     isclass,
     ismethod,
     )
+import logging
 import os
 from pprint import pformat
 import re
@@ -71,6 +72,7 @@
     BzrDir,
     format_registry,
     )
+from bzrlib import trace
 from bzrlib.transport import get_transport
 import fixtures
 import pytz
@@ -567,6 +569,13 @@
         self.factory = LaunchpadObjectFactory()
         self.direct_database_server = False
         self._use_bzr_branch_called = False
+        # XXX: JonathanLange 2010-12-24 bug=694140: Because of Launchpad's
+        # messing with global log state (see
+        # canonical.launchpad.scripts.logger), trace._bzr_logger does not
+        # necessarily equal logging.getLogger('bzr'), so we have to explicitly
+        # make it so in order to avoid "No handlers for "bzr" logger'
+        # messages.
+        trace._bzr_logger = logging.getLogger('bzr')
 
     def getUserBrowser(self, url=None, user=None, password='test'):
         """Return a Browser logged in as a fresh user, maybe opened at `url`.

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2010-12-17 13:11:38 +0000
+++ lib/lp_sitecustomize.py	2010-12-30 15:05:26 +0000
@@ -51,7 +51,9 @@
 
 def silence_bzr_logger():
     """Install the NullHandler on the bzr logger to silence logs."""
-    logging.getLogger('bzr').addHandler(NullHandler())
+    bzr_logger = logging.getLogger('bzr')
+    bzr_logger.addHandler(NullHandler())
+    bzr_logger.propagate = False
 
 
 def silence_zcml_logger():