Fix --no-color test

tests/test_main.py

@@ -12,8 +12,6 @@
 
 import sys
 
-import pytest
-
 from twine import __main__ as dunder_main
 
 
@@ -34,7 +32,6 @@ def test_exception_handling(monkeypatch, capsys):
     ]
 
 
-@pytest.mark.xfail(reason="capsys isn't reset, resulting in duplicate lines")
 def test_no_color_exception(monkeypatch, capsys):
     monkeypatch.setattr(sys, "argv", ["twine", "--no-color", "upload", "missing.whl"])
 

twine/__main__.py

@@ -22,27 +22,27 @@
 from twine import cli
 from twine import exceptions
 
+logger = logging.getLogger(__name__)
+
 
 def main() -> Any:
-    # Ensure that all log messages are displayed.
-    # Color will be configured during cli.dispatch() after argparse.
-    root_logger = logging.getLogger("twine")
-    root_logger.addHandler(logging.StreamHandler(sys.stdout))
+    # Ensure that all errors are logged, even before argparse
+    cli.configure_logging()
 
     try:
         error = cli.dispatch(sys.argv[1:])
     except requests.HTTPError as exc:
         error = True
         status_code = exc.response.status_code
         status_phrase = http.HTTPStatus(status_code).phrase
-        root_logger.error(
+        logger.error(
             f"{exc.__class__.__name__}: {status_code} {status_phrase} "
             f"from {exc.response.url}\n"
             f"{exc.response.reason}"
         )
     except exceptions.TwineException as exc:
         error = True
-        root_logger.error(f"{exc.__class__.__name__}: {exc.args[0]}")
+        logger.error(f"{exc.__class__.__name__}: {exc.args[0]}")
 
     return error
 

twine/cli.py

@@ -26,52 +26,57 @@
 
 args = argparse.Namespace()
 
-
-def list_dependencies_and_versions() -> List[Tuple[str, str]]:
-    requires = importlib_metadata.requires("twine")  # type: ignore[no-untyped-call] # python/importlib_metadata#288  # noqa: E501
-    deps = [requirements.Requirement(r).name for r in requires]
-    return [(dep, importlib_metadata.version(dep)) for dep in deps]  # type: ignore[no-untyped-call] # python/importlib_metadata#288  # noqa: E501
-
-
-def dep_versions() -> str:
-    return ", ".join(
-        "{}: {}".format(*dependency) for dependency in list_dependencies_and_versions()
-    )
+# This module attribute facilitates project-wide configuration and usage of Rich.
+# https://rich.readthedocs.io/en/latest/console.html
+console = rich.console.Console(
+    # Setting force_terminal makes testing easier by ensuring color codes.
+    # This could be based on FORCE_COLORS or PY_COLORS in os.environ, but Rich doesn't
+    # support that (https://github.com/Textualize/rich/issues/343), and since this is
+    # a module attribute, os.environ would be read on import, which complicates testing.
+    # no_color is set in dispatch() after argparse.
+    force_terminal=True,
+    theme=rich.theme.Theme(
+        {
+            "logging.level.debug": "green",
+            "logging.level.info": "blue",
+            "logging.level.warning": "yellow",
+            "logging.level.error": "red",
+            "logging.level.critical": "reverse red",
+        }
+    ),
+)
 
 
 def configure_logging() -> None:
     root_logger = logging.getLogger("twine")
 
-    # Overwrite basic configuration in main()
+    # This prevents failures test_main.py due to capsys not being cleared.
     # TODO: Use dictConfig() instead?
     for handler in root_logger.handlers:
         root_logger.removeHandler(handler)
 
     root_logger.addHandler(
         rich.logging.RichHandler(
-            # TODO: Maybe make console a module attribute to facilitate testing and
-            # using Rich's other functionality.
-            console=rich.console.Console(
-                # TODO: Set this if FORCE_COLOR or PY_COLORS in os.environ
-                force_terminal=True,
-                no_color=getattr(args, "no_color", False),
-                theme=rich.theme.Theme(
-                    {
-                        "logging.level.debug": "green",
-                        "logging.level.info": "blue",
-                        "logging.level.warning": "yellow",
-                        "logging.level.error": "red",
-                        "logging.level.critical": "reverse red",
-                    }
-                ),
-            ),
+            console=console,
             show_time=False,
             show_path=False,
             highlighter=rich.highlighter.NullHighlighter(),
         )
     )
 
 
+def list_dependencies_and_versions() -> List[Tuple[str, str]]:
+    requires = importlib_metadata.requires("twine")  # type: ignore[no-untyped-call] # python/importlib_metadata#288  # noqa: E501
+    deps = [requirements.Requirement(r).name for r in requires]
+    return [(dep, importlib_metadata.version(dep)) for dep in deps]  # type: ignore[no-untyped-call] # python/importlib_metadata#288  # noqa: E501
+
+
+def dep_versions() -> str:
+    return ", ".join(
+        "{}: {}".format(*dependency) for dependency in list_dependencies_and_versions()
+    )
+
+
 def dispatch(argv: List[str]) -> Any:
     registered_commands = importlib_metadata.entry_points(
         group="twine.registered_commands"
@@ -101,7 +106,10 @@ def dispatch(argv: List[str]) -> Any:
     )
     parser.parse_args(argv, namespace=args)
 
-    configure_logging()
+    # HACK: This attribute isn't documented, but this is an expedient way to alter the
+    # Rich configuration after argparse, while allowing logging to be configured on
+    # startup, ensuring all errors are displayed.
+    console.no_color = args.no_color
 
     main = registered_commands[args.command].load()