Merge lp:~frankban/launchpad/bug-993510 into lp:launchpad

Proposed by Francesco Banconi
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 15318
Proposed branch: lp:~frankban/launchpad/bug-993510
Merge into: lp:launchpad
Diff against target: 51 lines (+18/-1)
2 files modified
lib/lp/testing/__init__.py (+1/-1)
lib/lp/testing/tests/test_testcase.py (+17/-0)
To merge this branch: bzr merge lp:~frankban/launchpad/bug-993510
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+107630@code.launchpad.net

Description of the change

= Summary =

Log handlers are removed before each test because tests should not depend on global logging config.

== Changes ==

Iterate over a copy of the handler list, so that the original list is not affected by the .remove calls and all handlers are correctly removed.

NO QA

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/__init__.py

== Tests ==

$ bin/test -vvc --load-list ../s6
Running tests at level 1
Running lp.testing.layers.LaunchpadZopelessLayer tests:
  Set up lp.testing.layers.BaseLayer in 0.052 seconds.
  Set up lp.testing.layers.ZopelessLayer in 3.405 seconds.
  Set up lp.testing.layers.DatabaseLayer in 1.217 seconds.
  Set up lp.testing.layers.LibrarianLayer in 5.545 seconds.
  Set up lp.testing.layers.MemcachedLayer in 0.113 seconds.
  Set up lp.testing.layers.RabbitMQLayer in 1.501 seconds.
  Set up lp.testing.layers.LaunchpadLayer in 0.000 seconds.
  Set up lp.testing.layers.LaunchpadScriptLayer in 0.003 seconds.
  Set up lp.testing.layers.LaunchpadZopelessLayer in 0.000 seconds.
  Running:
 lp.archivepublisher.tests.test_publish_ftpmaster.TestPublishFTPMasterScript.test_script_is_happy_with_no_publications
 lp.services.job.tests.test_runner.TestJobRunner.test_runJobHandleErrors_oops_generated_user_notify_failsNo handlers could be found for logger "root"

  Ran 2 tests with 0 failures and 0 errors in 2.726 seconds.
Tearing down left over layers:
  Tear down lp.testing.layers.LaunchpadZopelessLayer in 0.000 seconds.
  Tear down lp.testing.layers.LaunchpadScriptLayer in 0.001 seconds.
  Tear down lp.testing.layers.ZopelessLayer ... not supported
  Tear down lp.testing.layers.LaunchpadLayer in 0.000 seconds.
  Tear down lp.testing.layers.LibrarianLayer in 0.137 seconds.
  Tear down lp.testing.layers.MemcachedLayer in 0.001 seconds.
  Tear down lp.testing.layers.RabbitMQLayer ... not supported
  Tear down lp.testing.layers.DatabaseLayer in 0.162 seconds.
  Tear down lp.testing.layers.BaseLayer in 0.001 seconds.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Looks good. I see why this would be a problem.

I wonder if we should have a test for this behaviour.

review: Approve (code)
Revision history for this message
Francesco Banconi (frankban) wrote :

Thanks Benji, added a test as you suggested.

$ bin/test -cvvt lp.testing.tests.test_testcase.TestRemoveLoggingHandlers
Running tests at level 1
Running zope.testing.testrunner.layer.UnitTests tests:
  Set up zope.testing.testrunner.layer.UnitTests in 0.000 seconds.
  Running:
 lp.testing.tests.test_testcase.TestRemoveLoggingHandlers.test_handlers_list_is_empty
  Ran 1 tests with 0 failures and 0 errors in 0.001 seconds.
Tearing down left over layers:
  Tear down zope.testing.testrunner.layer.UnitTests in 0.000 seconds.

Revision history for this message
Benji York (benji) wrote :

The test looks good. It is sort of funny (in a good way) how the test machinery is part of the test itself.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/testing/__init__.py'
2--- lib/lp/testing/__init__.py 2012-05-28 08:44:41 +0000
3+++ lib/lp/testing/__init__.py 2012-05-28 15:45:24 +0000
4@@ -653,7 +653,7 @@
5 # Remove all log handlers, tests should not depend on global logging
6 # config but should make their own config instead.
7 logger = logging.getLogger()
8- for handler in logger.handlers:
9+ for handler in list(logger.handlers):
10 logger.removeHandler(handler)
11
12 def assertStatementCount(self, expected_count, function, *args, **kwargs):
13
14=== modified file 'lib/lp/testing/tests/test_testcase.py'
15--- lib/lp/testing/tests/test_testcase.py 2012-01-01 18:15:02 +0000
16+++ lib/lp/testing/tests/test_testcase.py 2012-05-28 15:45:24 +0000
17@@ -5,6 +5,7 @@
18
19 __metaclass__ = type
20
21+import logging
22 from StringIO import StringIO
23 import sys
24
25@@ -16,6 +17,7 @@
26 from lp.services.webapp import errorlog
27 from lp.testing import (
28 record_statements,
29+ TestCase,
30 TestCaseWithFactory,
31 )
32 from lp.testing.layers import (
33@@ -102,3 +104,18 @@
34 # rfc822 serializer is lossy).
35 oops_report = self.oopses[0]
36 self.assertEqual(from_details['id'], oops_report['id'])
37+
38+
39+class TestRemoveLoggingHandlers(TestCase):
40+
41+ def setUp(self):
42+ self.logger = logging.getLogger()
43+ # Add 2 handlers.
44+ self.logger.addHandler(logging.Handler())
45+ self.logger.addHandler(logging.Handler())
46+ # `TestCase.setUp()` removes the handlers just added.
47+ super(TestRemoveLoggingHandlers, self).setUp()
48+
49+ def test_handlers_list_is_empty(self):
50+ # Ensure `TestCase.setUp()` correctly removed all logging handlers.
51+ self.assertEqual(0, len(self.logger.handlers))