cli: Introduce new --format argument (and remove --xunit-file)

Any output (text or xUnit) is now directed to the standard output. It
will be easier to add new formats (such as CSV or JSON, which I plan).

CHANGES.rst

@@ -1,7 +1,14 @@
-0.8.11 (unreleased)
+0.9.0 (unreleased)
 -------------------
 
-- Nothing changed yet.
+- |backward-incompatible| Remove ``--xunit-file`` argument from
+  ``check-branches`` and ``check-fixmes`` commands. It can be replaced
+  by a new ``--format=xunit`` argument and redirecting the standard
+  output to a file.
+
+  This change is needed to properly introduce the ``--format``
+  argument that controls the formatting output, which is now directed
+  to the standard output.
 
 
 0.8.10 (2023-08-16)

src/check_oldies/annotations.py

@@ -151,7 +151,7 @@ def get_annotations(config: Config):
         config.path, config.annotation_regex, config.whitelist
     ):
         filename, line_no, line_content = candidate.split(":", 2)
-        # FIXME (dbaty, 2020-10-21): it should be possible to apply
+        # FIXME (dbaty, 2024-03-28): it should be possible to apply
         # the right regex (with boundaries) directly in git grep.
         if config.py_annotation_regex.search(line_content):
             annotations.append(Annotation(filename, int(line_no), line_content))

src/check_oldies/branches.py

@@ -8,6 +8,7 @@
 
 from . import commands
 from . import githost
+from . import output
 
 
 TODAY = datetime.date.today()
@@ -40,8 +41,8 @@ class Config:
     path: str = "."
     max_age: int = 90
 
+    output_format: output.OutputFormat = output.OutputFormat.TEXT
     colorize_errors: bool = True
-    xunit_file: str = None
 
     calm_branches: typing.Sequence = ("gh-pages", "master", "main", "prod", "maint(enance)?/.*")
     ignore_branches_without_pull_request: bool = False
@@ -96,6 +97,11 @@ def name_and_details(self):
             details += f", linked to {pr.state} PR/MR #{pr.number} ({pr.url})"
         return details
 
+    def to_text(self):
+        return (
+            f"{self.author[:30]: <30} - {self.age: >4} days - {self.name_and_details}"
+        )
+
 
 def get_repository_info(path):
     """Extract the repository owner and name from the origin remote."""
@@ -131,12 +137,12 @@ def get_branches(config: Config):
         branch = branch.strip()[len("origin/") :]
         if config.ignore_branch(branch) or "->" in branch:
             continue
-        output = commands.get_output(
+        out = commands.get_output(
             ("git", "log", f"origin/{branch}", "-1", "--format=%ae %ci"),
             cwd=config.path,
         )[0]
         # line looks like "[email protected] 2018-12-19 14:18:52 +0100"
-        email, date, *_rest = output.split(" ")
+        email, date, *_rest = out.split(" ")
         date = datetime.date(*[int(s) for s in date.split("-")])
         age = (TODAY - date).days
         branches.append(
@@ -151,7 +157,7 @@ def get_branches(config: Config):
         )
 
     if not branches:
-        return ()
+        return []
 
     if config.host_api_access:
         pr_getter = githost.PullRequestGetter(config.platform, config.host_owner, config.host_api_access)

src/check_oldies/check_branches.py

@@ -4,7 +4,7 @@
 
 from . import branches
 from . import configuration
-from . import xunit
+from . import output
 
 
 def get_parser():
@@ -18,6 +18,14 @@ def get_parser():
             f"Defaults to {configuration.PYPROJECT_FILENAME} if it exists."
         ),
     )
+    parser.add_argument(
+        "--format",
+        default=output.OutputFormat.TEXT,
+        dest="output_format",
+        help="Output format. Defaults to human-readable text (one result per line).",
+        choices=sorted(output.OutputFormat),
+        type=output.OutputFormat,
+    )
     parser.add_argument(
         "path",
         nargs="?",
@@ -41,20 +49,9 @@ def get_parser():
         dest="colorize_errors",
         help="Do not colorize errors. Defaults to colorizing errors in red.",
     )
-    parser.add_argument(
-        "--xunit-file",
-        action="store",
-        help="Path of the xUnit report file to write. Defaults to no xUnit output.",
-    )
     return parser
 
 
-def branch_str(branch):
-    return (
-        f"{branch.author[:30]: <30} - {branch.age: >4} days - {branch.name_and_details}"
-    )
-
-
 def main():
     parser = get_parser()
     config = configuration.get_config(
@@ -63,44 +60,27 @@ def main():
     if not configuration.is_git_directory(config.path):
         sys.exit(f'Invalid path: "{config.path}" is not a Git repository.')
 
-    if config.colorize_errors:
-        warn = "\033[91m{}\033[0m".format
-    else:
-        warn = lambda text: text  # pylint: disable=unnecessary-lambda-assignment
-
-    all_branches = branches.get_branches(config)
+    branch_results = branches.get_branches(config)
+    branch_results.sort(key=lambda branch: (branch.author, -branch.age, branch.name))
+    has_old_branches = any(branch for branch in branch_results if branch.is_old)
 
-    out = []
-    uncolorized_out = []
-    for branch in sorted(
-        all_branches, key=lambda branch: (branch.author, -branch.age, branch.name)
-    ):
-        line = branch_str(branch)
-        out.append(warn(line) if branch.is_old else line)
-        uncolorized_out.append(line if branch.is_old else line)
-    has_old_branches = any(branch for branch in all_branches if branch.is_old)
-
-    out = os.linesep.join(out)
     if has_old_branches:
         err_msg = "NOK: Some branches are too old."
-        print(err_msg)
+        ok_msg = ""
     else:
         err_msg = ""
-        print("OK: All branches are fresh.")
-    if out:
-        print(out)
+        ok_msg = "OK: All branches are fresh."
 
-    if config.xunit_file:
-        uncolorized_out = os.linesep.join(uncolorized_out)
-        xunit.create_xunit_file(
-            os.path.abspath(config.xunit_file),
-            "check-branches",
-            "branches",
-            "CheckBranches",
-            err_msg=err_msg,
-            stdout=uncolorized_out,
-            stderr="",
-        )
+    output.printer(
+        branch_results,
+        config.output_format,
+        ok_message=ok_msg,
+        error_message=err_msg,
+        colorize_errors=config.colorize_errors,
+        xunit_suite_name="check-branches",
+        xunit_case_name="branches",
+        xunit_class_name="CheckBranches",
+    )
 
     sys.exit(os.EX_DATAERR if has_old_branches else os.EX_OK)
 

src/check_oldies/commands.py

@@ -1,5 +1,9 @@
 import os
-import pipes
+# FIXME (dbaty, 2024-03-28): `pipes` is deprecated since Python 3.11,
+# will be removed in 3.13. We should use `subprocess` instead (but
+# we'll have to make sure that we can do that and still support 3.8
+# and 3.9).
+import pipes  # pylint: disable=deprecated-module
 import subprocess
 import tempfile
 

src/check_oldies/compat.py

@@ -0,0 +1,38 @@
+import enum
+
+
+__all__ = ["StrEnum"]
+
+try:
+    from enum import StrEnum
+except ImportError:  # Python < 3.11
+    # fmt: off
+    # pylint: disable=consider-using-f-string
+    # Lifted from Python standard library, with docstrings removed for brevity.
+    # https://github.com/python/cpython/blob/c1712ef066321c01bf09cba3f22fc474b5b8dfa7/Lib/enum.py
+    class StrEnum(str, enum.Enum):
+        def __new__(cls, *values):
+            "values must already be of type `str`"
+            if len(values) > 3:
+                raise TypeError('too many arguments for str(): %r' % (values, ))
+            if len(values) == 1:
+                # it must be a string
+                if not isinstance(values[0], str):
+                    raise TypeError('%r is not a string' % (values[0], ))
+            if len(values) >= 2:
+                # check that encoding argument is a string
+                if not isinstance(values[1], str):
+                    raise TypeError('encoding must be a string, not %r' % (values[1], ))
+            if len(values) == 3:
+                # check that errors argument is a string
+                if not isinstance(values[2], str):
+                    raise TypeError('errors must be a string, not %r' % (values[2]))
+            value = str(*values)
+            member = str.__new__(cls, value)
+            member._value_ = value
+            return member
+
+        @staticmethod
+        def _generate_next_value_(name, start, count, last_values):
+            return name.lower()
+    # fmt: on

src/check_oldies/output.py

@@ -0,0 +1,90 @@
+import enum
+import os
+import typing
+import xml.etree.ElementTree
+
+from . import compat
+
+
+class OutputFormat(compat.StrEnum):
+    TEXT = enum.auto()
+    XUNIT = enum.auto()
+
+
+def text_formatter(
+    objects: list,
+    ok_message: str,
+    error_message: str,
+    colorize_errors=True,
+    **unsupported_options,
+) -> str:
+    if colorize_errors:
+        warn = "\033[91m{}\033[0m".format
+    else:
+        warn = lambda text: text  # pylint: disable=unnecessary-lambda-assignment
+
+    lines = []
+    if ok_message:
+        lines.append(ok_message)
+    if error_message:
+        lines.append(warn(error_message))
+
+    for obj in objects:
+        lines.append(warn(obj.to_text()) if obj.is_old else obj.to_text())
+    return os.linesep.join(lines)
+
+
+def xunit_formatter(
+    objects: list,
+    error_message: str,
+    xunit_suite_name: str,
+    xunit_case_name: str,
+    xunit_class_name: str,
+    **unsupported_options,
+) -> str:
+    stdout = os.linesep.join(obj.to_text() for obj in objects)
+    suite = xml.etree.ElementTree.Element(
+        "testsuite",
+        {
+            "name": xunit_suite_name,
+            "tests": "1",
+            "errors": "0",
+            "failures": "1" if error_message else "0",
+        },
+    )
+    case = xml.etree.ElementTree.SubElement(
+        suite,
+        "testcase",
+        {"classname": xunit_class_name, "name": xunit_case_name},
+    )
+    if error_message:
+        failure = xml.etree.ElementTree.SubElement(
+            case,
+            "failure",
+            {"message": error_message},
+        )
+        failure.text = stdout
+
+    system_out = xml.etree.ElementTree.SubElement(case, "system-out")
+    system_out.text = stdout
+
+    # FIXME (dbaty, 2024-03-28): this element is always empty and
+    # could be removed (because the schema says it's optional).
+    system_err = xml.etree.ElementTree.SubElement(case, "system-err")
+    system_err.text = ""
+
+    return xml.etree.ElementTree.tostring(suite, encoding="utf-8").decode("utf-8")
+
+
+def get_formatter(format_) -> typing.Callable:
+    if format_ == OutputFormat.TEXT:
+        return text_formatter
+    if format_ == OutputFormat.XUNIT:
+        return xunit_formatter
+    raise ValueError(f"Unknown output format: '{format_}'")
+
+
+def printer(objects: list, output_format, **options):
+    formatter = get_formatter(output_format)
+    formatted = formatter(objects, **options)
+    print(formatted)

tests/test_check_branches.py

@@ -10,6 +10,7 @@
 from check_oldies import branches
 from check_oldies import check_branches
 from check_oldies import commands
+from check_oldies import output
 
 from . import base
 
@@ -113,17 +114,17 @@ def test_output_old_branches(capfd):  # capfd is a pytest fixture
     assert stdout == expected
 
 
-def test_xunit_file_generation(tmp_path):  # tmp_path is a pytest fixture
-    xunit_path = tmp_path / "xunit.xml"
+def test_xunit_file_generation(capfd):  # capfd is a pytest fixture
     config = branches.Config(
         path=base.TEST_DIR_PATH.parent,
         max_age=9999,
-        xunit_file=xunit_path,
+        output_format=output.OutputFormat.XUNIT,
     )
 
     with mock.patch("check_oldies.configuration.get_config", return_value=config):
         with pytest.raises(SystemExit) as caught_exit:
             check_branches.main()
+    captured = capfd.readouterr()
 
     assert caught_exit.value.code == 0
-    assert 'failures="0"' in xunit_path.read_text()
+    assert 'failures="0"' in captured.out