Change signature of packaging.tests.support.LoggingCatcher.get_logs.

I need this for some tests, and it makes code clearer.  This commit also
changes some assertEqual calls to use (actual, expected) order and fix
some pyflakes warnings.

Lib/packaging/tests/support.py

@@ -82,10 +82,13 @@ class LoggingCatcher:
     configured to record all messages logged to the 'packaging' logger.
 
     Use get_logs to retrieve messages and self.loghandler.flush to discard
-    them.  get_logs automatically flushes the logs; if you test code that
+    them.  get_logs automatically flushes the logs, unless you pass
-    generates logging messages but don't use get_logs, you have to flush
+    *flush=False*, for example to make multiple calls to the method with
-    manually before doing other checks on logging message, otherwise you
+    different level arguments.  If your test calls some code that generates
-    will get irrelevant results.  See example in test_command_check.
+    logging message and then you don't call get_logs, you will need to flush
+    manually before testing other code in the same test_* method, otherwise
+    get_logs in the next lines will see messages from the previous lines.
+    See example in test_command_check.
     """
 
     def setUp(self):
@@ -109,25 +112,23 @@ class LoggingCatcher:
         logger2to3.setLevel(self._old_levels[1])
         super(LoggingCatcher, self).tearDown()
 
-    def get_logs(self, *levels):
+    def get_logs(self, level=logging.WARNING, flush=True):
-        """Return all log messages with level in *levels*.
+        """Return all log messages with given level.
 
-        Without explicit levels given, returns all messages.  *levels* defaults
+        *level* defaults to logging.WARNING.
-        to all levels.  For log calls with arguments (i.e.
+
-        logger.info('bla bla %r', arg)), the messages will be formatted before
+        For log calls with arguments (i.e.  logger.info('bla bla %r', arg)),
-        being returned (e.g. "bla bla 'thing'").
+        the messages will be formatted before being returned (e.g. "bla bla
+        'thing'").
 
         Returns a list.  Automatically flushes the loghandler after being
-        called.
+        called, unless *flush* is False (this is useful to get e.g. all
-
+        warnings then all info messages).
-        Example: self.get_logs(logging.WARN, logging.DEBUG).
         """
-        if not levels:
+        messages = [log.getMessage() for log in self.loghandler.buffer
-            messages = [log.getMessage() for log in self.loghandler.buffer]
+                    if log.levelno == level]
-        else:
+        if flush:
-            messages = [log.getMessage() for log in self.loghandler.buffer
+            self.loghandler.flush()
-                        if log.levelno in levels]
-        self.loghandler.flush()
         return messages
 
 

Lib/packaging/tests/test_command_check.py

@@ -1,6 +1,5 @@
 """Tests for distutils.command.check."""
 
-import logging
 from packaging.command.check import check
 from packaging.metadata import _HAS_DOCUTILS
 from packaging.errors import PackagingSetupError, MetadataMissingError
@@ -27,11 +26,11 @@ class CheckTestCase(support.LoggingCatcher,
         # let's run the command with no metadata at all
         # by default, check is checking the metadata
         # should have some warnings
-        cmd = self._run()
+        self._run()
         # trick: using assertNotEqual with an empty list will give us a more
         # useful error message than assertGreater(.., 0) when the code change
         # and the test fails
-        self.assertNotEqual([], self.get_logs(logging.WARNING))
+        self.assertNotEqual(self.get_logs(), [])
 
         # now let's add the required fields
         # and run it again, to make sure we don't get
@@ -40,8 +39,8 @@ class CheckTestCase(support.LoggingCatcher,
                     'author_email': 'xxx',
                     'name': 'xxx', 'version': '4.2',
                     }
-        cmd = self._run(metadata)
+        self._run(metadata)
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
         # now with the strict mode, we should
         # get an error if there are missing metadata
@@ -53,8 +52,8 @@ class CheckTestCase(support.LoggingCatcher,
         self.loghandler.flush()
 
         # and of course, no error when all metadata fields are present
-        cmd = self._run(metadata, strict=True)
+        self._run(metadata, strict=True)
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
         # now a test with non-ASCII characters
         metadata = {'home_page': 'xxx', 'author': '\u00c9ric',
@@ -62,15 +61,15 @@ class CheckTestCase(support.LoggingCatcher,
                     'version': '1.2',
                     'summary': 'Something about esszet \u00df',
                     'description': 'More things about esszet \u00df'}
-        cmd = self._run(metadata)
+        self._run(metadata)
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
     def test_check_metadata_1_2(self):
         # let's run the command with no metadata at all
         # by default, check is checking the metadata
         # should have some warnings
-        cmd = self._run()
+        self._run()
-        self.assertNotEqual([], self.get_logs(logging.WARNING))
+        self.assertNotEqual(self.get_logs(), [])
 
         # now let's add the required fields and run it again, to make sure we
         # don't get any warning anymore let's use requires_python as a marker
@@ -80,8 +79,8 @@ class CheckTestCase(support.LoggingCatcher,
                     'name': 'xxx', 'version': '4.2',
                     'requires_python': '2.4',
                     }
-        cmd = self._run(metadata)
+        self._run(metadata)
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
         # now with the strict mode, we should
         # get an error if there are missing metadata
@@ -99,8 +98,8 @@ class CheckTestCase(support.LoggingCatcher,
 
         # now with correct version format again
         metadata['version'] = '4.2'
-        cmd = self._run(metadata, strict=True)
+        self._run(metadata, strict=True)
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
     @unittest.skipUnless(_HAS_DOCUTILS, "requires docutils")
     def test_check_restructuredtext(self):
@@ -109,9 +108,7 @@ class CheckTestCase(support.LoggingCatcher,
         pkg_info, dist = self.create_dist(description=broken_rest)
         cmd = check(dist)
         cmd.check_restructuredtext()
-        self.assertEqual(len(self.get_logs(logging.WARNING)), 1)
+        self.assertEqual(len(self.get_logs()), 1)
-        # clear warnings from the previous call
-        self.loghandler.flush()
 
         # let's see if we have an error with strict=1
         metadata = {'home_page': 'xxx', 'author': 'xxx',
@@ -126,7 +123,7 @@ class CheckTestCase(support.LoggingCatcher,
         dist = self.create_dist(description='title\n=====\n\ntest \u00df')[1]
         cmd = check(dist)
         cmd.check_restructuredtext()
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
     def test_check_all(self):
         self.assertRaises(PackagingSetupError, self._run,
@@ -143,18 +140,18 @@ class CheckTestCase(support.LoggingCatcher,
         }
         cmd = check(dist)
         cmd.check_hooks_resolvable()
-        self.assertEqual(len(self.get_logs(logging.WARNING)), 1)
+        self.assertEqual(len(self.get_logs()), 1)
 
     def test_warn(self):
         _, dist = self.create_dist()
         cmd = check(dist)
-        self.assertEqual([], self.get_logs())
+        self.assertEqual(self.get_logs(), [])
         cmd.warn('hello')
-        self.assertEqual(['check: hello'], self.get_logs())
+        self.assertEqual(self.get_logs(), ['check: hello'])
         cmd.warn('hello %s', 'world')
-        self.assertEqual(['check: hello world'], self.get_logs())
+        self.assertEqual(self.get_logs(), ['check: hello world'])
         cmd.warn('hello %s %s', 'beautiful', 'world')
-        self.assertEqual(['check: hello beautiful world'], self.get_logs())
+        self.assertEqual(self.get_logs(), ['check: hello beautiful world'])
 
 
 def test_suite():

Lib/packaging/tests/test_command_cmd.py

@@ -1,5 +1,6 @@
 """Tests for distutils.cmd."""
 import os
+import logging
 
 from packaging.command.cmd import Command
 from packaging.dist import Distribution
@@ -43,7 +44,7 @@ class CommandTestCase(support.LoggingCatcher,
 
         wanted = ["command options for 'MyCmd':", '  option1 = 1',
                   '  option2 = 1']
-        msgs = self.get_logs()
+        msgs = self.get_logs(logging.INFO)
         self.assertEqual(msgs, wanted)
 
     def test_ensure_string(self):

Lib/packaging/tests/test_command_sdist.py

@@ -2,7 +2,6 @@
 import os
 import zipfile
 import tarfile
-import logging
 
 from packaging.tests.support import requires_zlib
 
@@ -221,7 +220,7 @@ class SDistTestCase(support.TempdirManager,
         # with the check subcommand
         cmd.ensure_finalized()
         cmd.run()
-        warnings = self.get_logs(logging.WARN)
+        warnings = self.get_logs()
         self.assertEqual(len(warnings), 4)
 
         # trying with a complete set of metadata
@@ -230,13 +229,10 @@ class SDistTestCase(support.TempdirManager,
         cmd.ensure_finalized()
         cmd.metadata_check = False
         cmd.run()
-        warnings = self.get_logs(logging.WARN)
+        warnings = self.get_logs()
-        # removing manifest generated warnings
-        warnings = [warn for warn in warnings if
-                    not warn.endswith('-- skipping')]
-        # the remaining warnings are about the use of the default file list and
-        # the absence of setup.cfg
         self.assertEqual(len(warnings), 2)
+        self.assertIn('using default file list', warnings[0])
+        self.assertIn("'setup.cfg' file not found", warnings[1])
 
     def test_show_formats(self):
         __, stdout = captured_stdout(show_formats)

Lib/packaging/tests/test_command_test.py

@@ -2,7 +2,6 @@ import os
 import re
 import sys
 import shutil
-import logging
 import unittest as ut1
 import packaging.database
 
@@ -149,7 +148,7 @@ class TestTest(TempdirManager,
         phony_project = 'ohno_ohno-impossible_1234-name_stop-that!'
         cmd.tests_require = [phony_project]
         cmd.ensure_finalized()
-        logs = self.get_logs(logging.WARNING)
+        logs = self.get_logs()
         self.assertIn(phony_project, logs[-1])
 
     def prepare_a_module(self):

Lib/packaging/tests/test_command_upload_docs.py

@@ -1,6 +1,7 @@
 """Tests for packaging.command.upload_docs."""
 import os
 import shutil
+import logging
 import zipfile
 try:
     import _ssl
@@ -141,13 +142,16 @@ class UploadDocsTestCase(support.TempdirManager,
         self.pypi.default_response_status = '403 Forbidden'
         self.prepare_command()
         self.cmd.run()
-        self.assertIn('Upload failed (403): Forbidden', self.get_logs()[-1])
+        errors = self.get_logs(logging.ERROR)
+        self.assertEqual(len(errors), 1)
+        self.assertIn('Upload failed (403): Forbidden', errors[0])
 
         self.pypi.default_response_status = '301 Moved Permanently'
         self.pypi.default_response_headers.append(
             ("Location", "brand_new_location"))
         self.cmd.run()
-        self.assertIn('brand_new_location', self.get_logs()[-1])
+        lastlog = self.get_logs(logging.INFO)[-1]
+        self.assertIn('brand_new_location', lastlog)
 
     def test_reads_pypirc_data(self):
         self.write_file(self.rc, PYPIRC % self.pypi.full_address)
@@ -171,7 +175,7 @@ class UploadDocsTestCase(support.TempdirManager,
         self.prepare_command()
         self.cmd.show_response = True
         self.cmd.run()
-        record = self.get_logs()[-1]
+        record = self.get_logs(logging.INFO)[-1]
         self.assertTrue(record, "should report the response")
         self.assertIn(self.pypi.default_response_data, record)
 

Lib/packaging/tests/test_config.py

@@ -1,7 +1,6 @@
 """Tests for packaging.config."""
 import os
 import sys
-import logging
 from io import StringIO
 
 from packaging import command
@@ -375,15 +374,14 @@ class ConfigTestCase(support.TempdirManager,
         self.write_file('README', 'yeah')
         self.write_file('hooks.py', HOOKS_MODULE)
         self.get_dist()
-        logs = self.get_logs(logging.WARNING)
+        self.assertEqual(['logging_hook called'], self.get_logs())
-        self.assertEqual(['logging_hook called'], logs)
         self.assertIn('hooks', sys.modules)
 
     def test_missing_setup_hook_warns(self):
         self.write_setup({'setup-hooks': 'this.does._not.exist'})
         self.write_file('README', 'yeah')
         self.get_dist()
-        logs = self.get_logs(logging.WARNING)
+        logs = self.get_logs()
         self.assertEqual(1, len(logs))
         self.assertIn('cannot find setup hook', logs[0])
 
@@ -397,7 +395,7 @@ class ConfigTestCase(support.TempdirManager,
         dist = self.get_dist()
 
         self.assertEqual(['haven', 'first', 'third'], dist.py_modules)
-        logs = self.get_logs(logging.WARNING)
+        logs = self.get_logs()
         self.assertEqual(1, len(logs))
         self.assertIn('cannot find setup hook', logs[0])
 

Lib/packaging/tests/test_dist.py

@@ -1,7 +1,6 @@
 """Tests for packaging.dist."""
 import os
 import sys
-import logging
 import textwrap
 
 import packaging.dist
@@ -74,7 +73,7 @@ class DistributionTestCase(support.TempdirManager,
                             'version': '1.2',
                             'home_page': 'xxxx',
                             'badoptname': 'xxx'})
-        logs = self.get_logs(logging.WARNING)
+        logs = self.get_logs()
         self.assertEqual(len(logs), 1)
         self.assertIn('unknown argument', logs[0])
 
@@ -85,7 +84,7 @@ class DistributionTestCase(support.TempdirManager,
                                    'version': '1.2', 'home_page': 'xxxx',
                                    'options': {}})
 
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
         self.assertNotIn('options', dir(dist))
 
     def test_non_empty_options(self):

Lib/packaging/tests/test_manifest.py

@@ -1,7 +1,6 @@
 """Tests for packaging.manifest."""
 import os
 import re
-import logging
 from io import StringIO
 from packaging.errors import PackagingTemplateError
 from packaging.manifest import Manifest, _translate_pattern, _glob_to_re
@@ -37,10 +36,10 @@ class ManifestTestCase(support.TempdirManager,
         super(ManifestTestCase, self).tearDown()
 
     def assertNoWarnings(self):
-        self.assertEqual(self.get_logs(logging.WARNING), [])
+        self.assertEqual(self.get_logs(), [])
 
     def assertWarnings(self):
-        self.assertGreater(len(self.get_logs(logging.WARNING)), 0)
+        self.assertNotEqual(self.get_logs(), [])
 
     def test_manifest_reader(self):
         tmpdir = self.mkdtemp()
@@ -51,7 +50,7 @@ class ManifestTestCase(support.TempdirManager,
         manifest = Manifest()
         manifest.read_template(MANIFEST)
 
-        warnings = self.get_logs(logging.WARNING)
+        warnings = self.get_logs()
         # the manifest should have been read and 3 warnings issued
         # (we didn't provide the files)
         self.assertEqual(3, len(warnings))

Lib/packaging/tests/test_metadata.py

@@ -1,7 +1,6 @@
 """Tests for packaging.metadata."""
 import os
 import sys
-import logging
 from textwrap import dedent
 from io import StringIO
 
@@ -302,7 +301,7 @@ class MetadataTestCase(LoggingCatcher,
                           'name': 'xxx',
                           'version': 'xxx',
                           'home_page': 'xxxx'})
-        logs = self.get_logs(logging.WARNING)
+        logs = self.get_logs()
         self.assertEqual(1, len(logs))
         self.assertIn('not a valid version', logs[0])
 
@@ -418,7 +417,7 @@ class MetadataTestCase(LoggingCatcher,
         # XXX check PEP and see if 3 == 3.0
         metadata['Requires-Python'] = '>=2.6, <3.0'
         metadata['Requires-Dist'] = ['Foo (>=2.6, <3.0)']
-        self.assertEqual([], self.get_logs(logging.WARNING))
+        self.assertEqual(self.get_logs(), [])
 
     @unittest.skip('needs to be implemented')
     def test_requires_illegal(self):