style: cleanups after lcov, though more than just lcov

4 files changed, 172 insertions, 202 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index d9af1e44..e9ae1d6e 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -23,7 +23,7 @@ Unreleased
 ----------
 
 - Feature: Added the `lcov` command to generate reports in LCOV format.
-  Thanks, Bradley Burns.
+  Thanks, Bradley Burns. Closes `issue 587`_ and `issue 626`_.
 
 - Dropped support for Python 3.6, which reached end-of-life on 2021-12-23.
 
@@ -39,6 +39,8 @@ Unreleased
 
 - Releases now have MacOS arm64 wheels for Apple Silicon (fixes `issue 1288`_).
 
+.. _issue 587: https://github.com/nedbat/coveragepy/issues/587
+.. _issue 626: https://github.com/nedbat/coveragepy/issues/626
 .. _issue 883: https://github.com/nedbat/coveragepy/issues/883
 .. _issue 1288: https://github.com/nedbat/coveragepy/issues/1288
 .. _issue 1294: https://github.com/nedbat/coveragepy/issues/1294
diff --git a/coverage/cmdline.py b/coverage/cmdline.py
index 9f5c9ea8..ad19ef29 100644
--- a/coverage/cmdline.py
+++ b/coverage/cmdline.py
@@ -123,15 +123,15 @@ class Opts:
         metavar="OUTFILE",
         help="Write the JSON report to this file. Defaults to 'coverage.json'",
     )
+    output_lcov = optparse.make_option(
+        '-o', '', action='store', dest='outfile',
+        metavar="OUTFILE",
+        help="Write the LCOV report to this file. Defaults to 'coverage.lcov'",
+    )
     json_pretty_print = optparse.make_option(
         '', '--pretty-print', action='store_true',
         help="Format the JSON for human readers.",
     )
-    lcov = optparse.make_option(
-        '-o', '', action='store', dest='outfile',
-        metavar="OUTFILE",
-        help="Write the LCOV report to this file. Defaults to 'coverage.lcov'"
-    )
     parallel_mode = optparse.make_option(
         '-p', '--parallel-mode', action='store_true',
         help=(
@@ -423,7 +423,21 @@ CMDS = {
             Opts.show_contexts,
             ] + GLOBAL_ARGS,
         usage="[options] [modules]",
-        description="Generate a JSON report of coverage results."
+        description="Generate a JSON report of coverage results.",
+    ),
+
+    'lcov': CmdOptionParser(
+        "lcov",
+        [
+            Opts.fail_under,
+            Opts.ignore_errors,
+            Opts.include,
+            Opts.output_lcov,
+            Opts.omit,
+            Opts.quiet,
+        ] + GLOBAL_ARGS,
+        usage="[options] [modules]",
+        description="Generate an LCOV report of coverage results.",
     ),
 
     'report': CmdOptionParser(
@@ -442,7 +456,7 @@ CMDS = {
             Opts.skip_empty,
             ] + GLOBAL_ARGS,
         usage="[options] [modules]",
-        description="Report coverage statistics on modules."
+        description="Report coverage statistics on modules.",
     ),
 
     'run': CmdOptionParser(
@@ -461,7 +475,7 @@ CMDS = {
             Opts.timid,
             ] + GLOBAL_ARGS,
         usage="[options] <pyfile> [program options]",
-        description="Run a Python program, measuring code execution."
+        description="Run a Python program, measuring code execution.",
     ),
 
     'xml': CmdOptionParser(
@@ -476,22 +490,8 @@ CMDS = {
             Opts.skip_empty,
             ] + GLOBAL_ARGS,
         usage="[options] [modules]",
-        description="Generate an XML report of coverage results."
+        description="Generate an XML report of coverage results.",
     ),
-
-    'lcov': CmdOptionParser(
-        "lcov",
-        [
-            Opts.fail_under,
-            Opts.ignore_errors,
-            Opts.include,
-            Opts.lcov,
-            Opts.omit,
-            Opts.quiet,
-        ] + GLOBAL_ARGS,
-        usage="[options] [modules]",
-        description="Generate an LCOV report of coverage results."
-    )
 }
 
 
@@ -681,7 +681,6 @@ class CoverageScript:
                 outfile=options.outfile,
                 **report_args
                 )
-
         else:
             # There are no other possible actions.
             raise AssertionError
@@ -876,10 +875,10 @@ HELP_TOPICS = {
             help        Get help on using coverage.py.
             html        Create an HTML report.
             json        Create a JSON report of coverage results.
+            lcov        Create an LCOV report of coverage results.
             report      Report coverage stats on modules.
             run         Run a Python program and measure code execution.
             xml         Create an XML report of coverage results.
-            lcov        Create an LCOV report of coverage results.
 
         Use "{program_name} help <command>" for detailed help on any command.
     """,
diff --git a/coverage/lcovreport.py b/coverage/lcovreport.py
index 770f7a25..4dc73c29 100644
--- a/coverage/lcovreport.py
+++ b/coverage/lcovreport.py
@@ -20,7 +20,7 @@ class LcovReporter:
         self.config = self.coverage.config
 
     def report(self, morfs, outfile=None):
-        """Renders the full lcov report
+        """Renders the full lcov report.
 
         'morfs' is a list of modules or filenames
 
@@ -34,41 +34,42 @@ class LcovReporter:
             self.get_lcov(fr, analysis, outfile)
 
     def get_lcov(self, fr, analysis, outfile=None):
-        """Produces the lcov data for a single file
+        """Produces the lcov data for a single file.
 
-        get_lcov currently supports both line and branch coverage,
+        This currently supports both line and branch coverage,
         however function coverage is not supported.
-
         """
-
         outfile.write("TN:\n")
         outfile.write(f"SF:{fr.relative_filename()}\n")
         source_lines = fr.source().splitlines()
+
         for covered in sorted(analysis.executed):
-            # Note: Coveragepy currently only supports checking *if* a line has
-            # been executed, not how many times, so we set this to 1 for nice
-            # output even if it's technically incorrect
-
-            # The lines below calculate a 64 bit encoded md5 hash of the line
-            # corresponding to the DA lines in the lcov file,
-            # for either case of the line being covered or missed in Coveragepy
-            # The final two characters of the encoding ("==") are removed from
-            # the hash to allow genhtml to run on the resulting lcov file
+            # Note: Coverage.py currently only supports checking *if* a line
+            # has been executed, not how many times, so we set this to 1 for
+            # nice output even if it's technically incorrect.
+
+            # The lines below calculate a 64-bit encoded md5 hash of the line
+            # corresponding to the DA lines in the lcov file, for either case
+            # of the line being covered or missed in coverage.py. The final two
+            # characters of the encoding ("==") are removed from the hash to
+            # allow genhtml to run on the resulting lcov file.
             if source_lines:
-                line = source_lines[covered - 1].encode("utf-8")
+                line = source_lines[covered-1].encode("utf-8")
             else:
                 line = b""
-            hashed = str(base64.b64encode(md5(line).digest())[:-2], encoding="utf-8")
+            hashed = base64.b64encode(md5(line).digest()).decode().rstrip("=")
             outfile.write(f"DA:{covered},1,{hashed}\n")
+
         for missed in sorted(analysis.missing):
             assert source_lines
             line = source_lines[missed-1].encode("utf-8")
-            hashed = str(base64.b64encode(md5(line).digest())[:-2], encoding="utf-8")
+            hashed = base64.b64encode(md5(line).digest()).decode().rstrip("=")
             outfile.write(f"DA:{missed},0,{hashed}\n")
+
         outfile.write(f"LF:{len(analysis.statements)}\n")
         outfile.write(f"LH:{len(analysis.executed)}\n")
 
-        # More information dense branch coverage data
+        # More information dense branch coverage data.
         missing_arcs = analysis.missing_branch_arcs()
         executed_arcs = analysis.executed_branch_arcs()
         for block_number, block_line_number in enumerate(
@@ -78,14 +79,15 @@ class LcovReporter:
                 sorted(missing_arcs[block_line_number])
             ):
                 # The exit branches have a negative line number,
-                # this will not produce valid lcov, and so setting
+                # this will not produce valid lcov. Setting
                 # the line number of the exit branch to 0 will allow
-                # for valid lcov, while preserving the data
+                # for valid lcov, while preserving the data.
                 line_number = max(line_number, 0)
                 outfile.write(f"BRDA:{line_number},{block_number},{branch_number},-\n")
+
             # The start value below allows for the block number to be
             # preserved between these two for loops (stopping the loop from
-            # resetting the value of the block number to 0)
+            # resetting the value of the block number to 0).
             for branch_number, line_number in enumerate(
                 sorted(executed_arcs[block_line_number]),
                 start=len(missing_arcs[block_line_number]),
@@ -93,7 +95,7 @@ class LcovReporter:
                 line_number = max(line_number, 0)
                 outfile.write(f"BRDA:{line_number},{block_number},{branch_number},1\n")
 
-        # Summary of the branch coverage
+        # Summary of the branch coverage.
         if analysis.has_arcs():
             branch_stats = analysis.branch_stats()
             brf = sum(t for t, k in branch_stats.values())
diff --git a/tests/test_lcov.py b/tests/test_lcov.py
index fcb17f93..f31d8c17 100644
--- a/tests/test_lcov.py
+++ b/tests/test_lcov.py
@@ -4,10 +4,11 @@
 """Test LCOV-based summary reporting for coverage.py."""
 
 import textwrap
-import coverage
 
 from tests.coveragetest import CoverageTest
 
+import coverage
+
 
 class LcovTest(CoverageTest):
     """Tests of the LCOV reports from coverage.py."""
@@ -17,9 +18,7 @@ class LcovTest(CoverageTest):
         Helper for tests that handles the common ceremony so the tests can
         show the consequences of changes in the setup.
         """
-        self.make_file(
-            "main_file.py",
-            """\
+        self.make_file("main_file.py", """\
             #!/usr/bin/env python3
 
             def cuboid_volume(l):
@@ -27,13 +26,9 @@ class LcovTest(CoverageTest):
 
             def IsItTrue():
                 return True
+            """)
 
-        """,
-        )
-
-        self.make_file(
-            "test_file.py",
-            """\
+        self.make_file("test_file.py", """\
             #!/usr/bin/env python3
 
             from main_file import cuboid_volume
@@ -45,9 +40,7 @@ class LcovTest(CoverageTest):
                     self.assertAlmostEqual(cuboid_volume(1),1)
                     self.assertAlmostEqual(cuboid_volume(0),0)
                     self.assertAlmostEqual(cuboid_volume(5.5),166.375)
-
-        """,
-        )
+            """)
 
     def get_lcov_report_content(self, filename="coverage.lcov"):
         """Return the content of an LCOV report."""
@@ -58,9 +51,7 @@ class LcovTest(CoverageTest):
     def test_lone_file(self):
         """For a single file with a couple of functions, the lcov should cover
         the function definitions themselves, but not the returns."""
-        self.make_file(
-            "main_file.py",
-            """\
+        self.make_file("main_file.py", """\
             #!/usr/bin/env python3
 
             def cuboid_volume(l):
@@ -68,21 +59,18 @@ class LcovTest(CoverageTest):
 
             def IsItTrue():
                 return True
-
-        """,
-        )
-        expected_result = """\
-        TN:
-        SF:main_file.py
-        DA:3,1,7URou3io0zReBkk69lEb/Q
-        DA:6,1,ilhb4KUfytxtEuClijZPlQ
-        DA:4,0,Xqj6H1iz/nsARMCAbE90ng
-        DA:7,0,LWILTcvARcydjFFyo9qM0A
-        LF:4
-        LH:2
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+            """)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:main_file.py
+            DA:3,1,7URou3io0zReBkk69lEb/Q
+            DA:6,1,ilhb4KUfytxtEuClijZPlQ
+            DA:4,0,Xqj6H1iz/nsARMCAbE90ng
+            DA:7,0,LWILTcvARcydjFFyo9qM0A
+            LF:4
+            LH:2
+            end_of_record
+            """)
         self.assert_doesnt_exist(".coverage")
         cov = coverage.Coverage(source=["."])
         self.start_import_stop(cov, "main_file")
@@ -100,39 +88,36 @@ class LcovTest(CoverageTest):
         self.start_import_stop(cov, "test_file")
         cov.lcov_report()
         self.assert_exists("data.lcov")
-        expected_result = """\
-        TN:
-        SF:main_file.py
-        DA:3,1,7URou3io0zReBkk69lEb/Q
-        DA:6,1,ilhb4KUfytxtEuClijZPlQ
-        DA:4,0,Xqj6H1iz/nsARMCAbE90ng
-        DA:7,0,LWILTcvARcydjFFyo9qM0A
-        LF:4
-        LH:2
-        end_of_record
-        TN:
-        SF:test_file.py
-        DA:3,1,R5Rb4IzmjKRgY/vFFc1TRg
-        DA:4,1,E/tvV9JPVDhEcTCkgrwOFw
-        DA:6,1,GP08LPBYJq8EzYveHJy2qA
-        DA:7,1,MV+jSLi6PFEl+WatEAptog
-        DA:8,0,qyqd1mF289dg6oQAQHA+gQ
-        DA:9,0,nmEYd5F1KrxemgC9iVjlqg
-        DA:10,0,jodMK26WYDizOO1C7ekBbg
-        DA:11,0,LtxfKehkX8o4KvC5GnN52g
-        LF:8
-        LH:4
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:main_file.py
+            DA:3,1,7URou3io0zReBkk69lEb/Q
+            DA:6,1,ilhb4KUfytxtEuClijZPlQ
+            DA:4,0,Xqj6H1iz/nsARMCAbE90ng
+            DA:7,0,LWILTcvARcydjFFyo9qM0A
+            LF:4
+            LH:2
+            end_of_record
+            TN:
+            SF:test_file.py
+            DA:3,1,R5Rb4IzmjKRgY/vFFc1TRg
+            DA:4,1,E/tvV9JPVDhEcTCkgrwOFw
+            DA:6,1,GP08LPBYJq8EzYveHJy2qA
+            DA:7,1,MV+jSLi6PFEl+WatEAptog
+            DA:8,0,qyqd1mF289dg6oQAQHA+gQ
+            DA:9,0,nmEYd5F1KrxemgC9iVjlqg
+            DA:10,0,jodMK26WYDizOO1C7ekBbg
+            DA:11,0,LtxfKehkX8o4KvC5GnN52g
+            LF:8
+            LH:4
+            end_of_record
+            """)
         actual_result = self.get_lcov_report_content(filename="data.lcov")
         assert expected_result == actual_result
 
     def test_branch_coverage_one_file(self):
         """Test that the reporter produces valid branch coverage."""
-        self.make_file(
-            "main_file.py",
-            """\
+        self.make_file("main_file.py", """\
             #!/usr/bin/env python3
 
             def is_it_x(x):
@@ -140,39 +125,34 @@ class LcovTest(CoverageTest):
                     return x
                 else:
                     return False
-
-            """,
-        )
+            """)
         self.assert_doesnt_exist(".coverage")
         cov = coverage.Coverage(branch=True, source=".")
         self.start_import_stop(cov, "main_file")
         cov.lcov_report()
         self.assert_exists("coverage.lcov")
-        expected_result = """\
-        TN:
-        SF:main_file.py
-        DA:3,1,4MDXMbvwQ3L7va1tsphVzw
-        DA:4,0,MuERA6EYyZNpKPqoJfzwkA
-        DA:5,0,sAyiiE6iAuPMte9kyd0+3g
-        DA:7,0,W/g8GJDAYJkSSurt59Mzfw
-        LF:4
-        LH:1
-        BRDA:5,0,0,-
-        BRDA:7,0,1,-
-        BRF:2
-        BRH:0
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:main_file.py
+            DA:3,1,4MDXMbvwQ3L7va1tsphVzw
+            DA:4,0,MuERA6EYyZNpKPqoJfzwkA
+            DA:5,0,sAyiiE6iAuPMte9kyd0+3g
+            DA:7,0,W/g8GJDAYJkSSurt59Mzfw
+            LF:4
+            LH:1
+            BRDA:5,0,0,-
+            BRDA:7,0,1,-
+            BRF:2
+            BRH:0
+            end_of_record
+            """)
         actual_result = self.get_lcov_report_content()
         assert expected_result == actual_result
 
     def test_branch_coverage_two_files(self):
         """Test that valid branch coverage is generated
         in the case of two files."""
-        self.make_file(
-            "main_file.py",
-            """\
+        self.make_file("main_file.py", """\
             #!/usr/bin/env python3
 
             def is_it_x(x):
@@ -180,13 +160,9 @@ class LcovTest(CoverageTest):
                     return x
                 else:
                     return False
+            """)
 
-            """,
-        )
-
-        self.make_file(
-            "test_file.py",
-            """\
+        self.make_file("test_file.py", """\
             #!/usr/bin/env python3
 
             from main_file import *
@@ -196,45 +172,42 @@ class LcovTest(CoverageTest):
                 def test_is_it_x(self):
                     self.assertEqual(is_it_x(3), 3)
                     self.assertEqual(is_it_x(4), False)
-
-            """,
-        )
+            """)
         self.assert_doesnt_exist(".coverage")
         cov = coverage.Coverage(branch=True, source=".")
         self.start_import_stop(cov, "test_file")
         cov.lcov_report()
         self.assert_exists("coverage.lcov")
-        expected_result = """\
-        TN:
-        SF:main_file.py
-        DA:3,1,4MDXMbvwQ3L7va1tsphVzw
-        DA:4,0,MuERA6EYyZNpKPqoJfzwkA
-        DA:5,0,sAyiiE6iAuPMte9kyd0+3g
-        DA:7,0,W/g8GJDAYJkSSurt59Mzfw
-        LF:4
-        LH:1
-        BRDA:5,0,0,-
-        BRDA:7,0,1,-
-        BRF:2
-        BRH:0
-        end_of_record
-        TN:
-        SF:test_file.py
-        DA:3,1,9TxKIyoBtmhopmlbDNa8FQ
-        DA:4,1,E/tvV9JPVDhEcTCkgrwOFw
-        DA:6,1,C3s/c8C1Yd/zoNG1GnGexg
-        DA:7,1,9qPyWexYysgeKtB+YvuzAg
-        DA:8,0,LycuNcdqoUhPXeuXUTf5lA
-        DA:9,0,FPTWzd68bDx76HN7VHu1wA
-        LF:6
-        LH:4
-        BRDA:0,0,0,1
-        BRDA:7,0,1,1
-        BRF:2
-        BRH:2
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:main_file.py
+            DA:3,1,4MDXMbvwQ3L7va1tsphVzw
+            DA:4,0,MuERA6EYyZNpKPqoJfzwkA
+            DA:5,0,sAyiiE6iAuPMte9kyd0+3g
+            DA:7,0,W/g8GJDAYJkSSurt59Mzfw
+            LF:4
+            LH:1
+            BRDA:5,0,0,-
+            BRDA:7,0,1,-
+            BRF:2
+            BRH:0
+            end_of_record
+            TN:
+            SF:test_file.py
+            DA:3,1,9TxKIyoBtmhopmlbDNa8FQ
+            DA:4,1,E/tvV9JPVDhEcTCkgrwOFw
+            DA:6,1,C3s/c8C1Yd/zoNG1GnGexg
+            DA:7,1,9qPyWexYysgeKtB+YvuzAg
+            DA:8,0,LycuNcdqoUhPXeuXUTf5lA
+            DA:9,0,FPTWzd68bDx76HN7VHu1wA
+            LF:6
+            LH:4
+            BRDA:0,0,0,1
+            BRDA:7,0,1,1
+            BRF:2
+            BRH:2
+            end_of_record
+            """)
         actual_result = self.get_lcov_report_content()
         assert actual_result == expected_result
 
@@ -242,39 +215,34 @@ class LcovTest(CoverageTest):
         """Test that for a given branch that is only half covered,
         the block numbers remain the same, and produces valid lcov.
         """
-        self.make_file(
-            "main_file.py",
-            """\
+        self.make_file("main_file.py", """\
             something = True
 
             if something:
                 print("Yes, something")
             else:
                 print("No, nothing")
-
-            """,
-        )
+            """)
         self.assert_doesnt_exist(".coverage")
         cov = coverage.Coverage(branch=True, source=".")
         self.start_import_stop(cov, "main_file")
         cov.lcov_report()
         self.assert_exists("coverage.lcov")
-        expected_result = """\
-        TN:
-        SF:main_file.py
-        DA:1,1,N4kbVOlkNI1rqOfCArBClw
-        DA:3,1,CmlqqPf0/H+R/p7/PLEXZw
-        DA:4,1,rE3mWnpoMq2W2sMETVk/uQ
-        DA:6,0,+Aov7ekIts7C96udNDVIIQ
-        LF:4
-        LH:3
-        BRDA:6,0,0,-
-        BRDA:4,0,1,1
-        BRF:2
-        BRH:1
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:main_file.py
+            DA:1,1,N4kbVOlkNI1rqOfCArBClw
+            DA:3,1,CmlqqPf0/H+R/p7/PLEXZw
+            DA:4,1,rE3mWnpoMq2W2sMETVk/uQ
+            DA:6,0,+Aov7ekIts7C96udNDVIIQ
+            LF:4
+            LH:3
+            BRDA:6,0,0,-
+            BRDA:4,0,1,1
+            BRF:2
+            BRH:1
+            end_of_record
+            """)
         actual_result = self.get_lcov_report_content()
         assert actual_result == expected_result
 
@@ -293,16 +261,15 @@ class LcovTest(CoverageTest):
         self.start_import_stop(cov, "__init__")
         cov.lcov_report()
         self.assert_exists("coverage.lcov")
-        expected_result = """\
-        TN:
-        SF:__init__.py
-        DA:1,1,1B2M2Y8AsgTpgAmY7PhCfg
-        LF:0
-        LH:1
-        BRF:0
-        BRH:0
-        end_of_record
-        """
-        expected_result = textwrap.dedent(expected_result)
+        expected_result = textwrap.dedent("""\
+            TN:
+            SF:__init__.py
+            DA:1,1,1B2M2Y8AsgTpgAmY7PhCfg
+            LF:0
+            LH:1
+            BRF:0
+            BRH:0
+            end_of_record
+            """)
         actual_result = self.get_lcov_report_content()
         assert actual_result == expected_result