Repository: beeware/briefcase
Pull request #1267 (2023-05-08 17:38:43)

Disable progress bar and wait bar if `stdout` is not attached to a tty
## Changes
- Disables the Progress Bar and Wait Bar if `stdout` if not attached to a tty

## Notes
- It was discovered in beeware/.github#36 that when `FORCE_COLOR` is set, Rich not only enables ANSI color and styling, but also turns on "live" elements like the Progress Bar
- Allowing the Progress Bar to run in CI can create really ugly [logs](https://github.com/beeware/.github/actions/runs/4902657729/jobs/8754641453)
- I initially considered disabling these dynamic elements when the `--no-input` switch is used....but I decided against it because:
  - It's a weak proxy for "non-interactive session"
  - It can disable the dynamic elements when they are still desired and reasonable to show
  - When users encounter the problem of polluted CI logs, they would be required to add an un-intuitive switch to _all_ invocations of Briefcase
- Rich's [recommendation](https://rich.readthedocs.io/en/stable/console.html#interactive-mode) for this situation is to instantiate `Console` with `force_terminal=True` and `force_interactive=False`
  - However, this recommendation is qualified with "you might want to do this in some CI environments"....and this only seems to make sense if the application using Rich is only really used in CI by the application developer
  - In Briefcase's case, we would need to establish some process for users to implement in their CI in order to activate these overrides for `Console`
  - Alternatively, we could always connect `sys.stdout.isatty()` to these overrides....although, I think I'd rather just let Rich do what it does by default and just target our known issues
    - After all, Rich has all the battle-tested logic for when terminal and interactive things should happen
- Thus, just disabling the specific uses of Rich's dynamic elements in Briefcase when a tty isn't available seems like the best way to use default functionality while avoiding known issues

## PR Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
- [x] All new features have been tested
- [x] All new features have been documented
- [x] I have read the **CONTRIBUTING.md** file
- [x] I will abide by the code of conduct


Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 62db8b03..5da9549c 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -14,12 +14,12 @@ repos:
       - id: isort
         additional_dependencies: [toml]
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.4.0
+    rev: v3.3.2
     hooks:
       - id: pyupgrade
         args: [--py38-plus]
   - repo: https://github.com/PyCQA/docformatter
-    rev: v1.6.5
+    rev: v1.6.4
     hooks:
       - id: docformatter
         args: [--in-place, --wrap-descriptions=88, --wrap-summaries=88]
diff --git a/changes/1264.misc.rst b/changes/1264.misc.rst
deleted file mode 100644
index 248da6ab..00000000
--- a/changes/1264.misc.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-Updated ``pre-commit`` hooks to the latest version.
diff --git a/changes/1265.misc.rst b/changes/1265.misc.rst
deleted file mode 100644
index b52526fc..00000000
--- a/changes/1265.misc.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-Updated pre-commit from 3.2.2 to 3.3.1.
diff --git a/changes/1267.bugfix.rst b/changes/1267.bugfix.rst
new file mode 100644
index 00000000..2c08eb7e
--- /dev/null
+++ b/changes/1267.bugfix.rst
@@ -0,0 +1,1 @@
+Briefcase's Progress Bar and Wait Bar will no longer be rendered if the ``FORCE_COLOR`` environment variable is set.
diff --git a/setup.cfg b/setup.cfg
index f54efe81..3bebf580 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -91,7 +91,7 @@ install_requires =
 # ensure environment consistency.
 dev =
     coverage[toml] == 7.2.5
-    pre-commit == 3.3.1
+    pre-commit == 3.2.2
     pytest == 7.3.1
     pytest-xdist == 3.2.1
     setuptools_scm[toml] == 7.1.0
diff --git a/src/briefcase/console.py b/src/briefcase/console.py
index 315bc2e3..37c05afb 100644
--- a/src/briefcase/console.py
+++ b/src/briefcase/console.py
@@ -348,23 +348,29 @@ class Log:
 
 class Console:
     def __init__(self, printer=Printer(), enabled=True):
+        self.enabled = enabled
         self.print = printer
+        # Use Rich's input() to read from user
         self.input = printer.console.input
-        self.enabled = enabled
         self._wait_bar: Progress = None
-        # Signal that Rich is dynamically controlling the console output.
-        # Therefore, all output must be printed to the screen by Rich to
-        # prevent corruption of dynamic elements like the Wait Bar.
+        # Signal that Rich is dynamically controlling the console output. Therefore,
+        # all output must be printed to the screen by Rich to prevent corruption of
+        # dynamic elements like the Wait Bar.
         self.is_console_controlled = False
 
-    def prompt(self, *values, markup=False, **kwargs):
-        """Print to the screen for soliciting user interaction.
-
-        :param values: strings to print as the user prompt
-        :param markup: True if prompt contains Rich markup
+    @property
+    def is_interactive(self):
+        """Returns interactivity mode based on the presence of a tty for stdout.
+
+        In a non-interactive session, Rich's dynamic elements like progress bars should
+        be disabled. By default, Rich will not render dynamic elements if a tty is not
+        available. However, environment variables like FORCE_COLOR can override this
+        behavior in Rich to not only show ANSI color and style but also dynamic
+        elements. In a CI environment with this configuration, a long-lived progress bar
+        can dramatically compromise the quality of logged output. So, dynamic elements
+        should be specifically disabled in non-interactive sessions.
         """
-        if self.enabled:
-            self.print(*values, markup=markup, stack_offset=4, **kwargs)
+        return sys.stdout.isatty()
 
     def progress_bar(self):
         """Returns a progress bar as a context manager."""
@@ -375,6 +381,7 @@ class Console:
             TextColumn("{task.percentage:>3.1f}%", style="default"),
             TextColumn("•", style="default"),
             TimeRemainingColumn(compact=True, elapsed_when_finished=True),
+            disable=not self.is_interactive,
             console=self.print.console,
         )
 
@@ -406,6 +413,7 @@ class Console:
                 BarColumn(bar_width=20, style="black", pulse_style="white"),
                 TextColumn("{task.fields[message]}"),
                 transient=True,
+                disable=not self.is_interactive,
                 console=self.print.console,
             )
             # start=False causes the progress bar to "pulse"
@@ -461,6 +469,15 @@ class Console:
             if is_wait_bar_running:
                 self._wait_bar.start()
 
+    def prompt(self, *values, markup=False, **kwargs):
+        """Print to the screen for soliciting user interaction.
+
+        :param values: strings to print as the user prompt
+        :param markup: True if prompt contains Rich markup
+        """
+        if self.enabled:
+            self.print(*values, markup=markup, stack_offset=4, **kwargs)
+
     def boolean_input(self, question, default=False):
         """Get a boolean input from user, in the form of y/n.
 


Test patch:
diff --git a/tests/console/Console/test_progress_bar.py b/tests/console/Console/test_progress_bar.py
new file mode 100644
index 00000000..6ca7a92e
--- /dev/null
+++ b/tests/console/Console/test_progress_bar.py
@@ -0,0 +1,10 @@
+def test_wait_bar_always_interactive(interactive_console):
+    """Progress Bar is not disabled when console is interactive."""
+    with interactive_console.progress_bar() as bar:
+        assert bar.disable is False
+
+
+def test_wait_bar_non_interactive(non_interactive_console):
+    """Progress Bar is disabled when console is non-interactive."""
+    with non_interactive_console.progress_bar() as bar:
+        assert bar.disable is True
diff --git a/tests/console/Console/test_properties.py b/tests/console/Console/test_properties.py
index 84189686..11afc6a3 100644
--- a/tests/console/Console/test_properties.py
+++ b/tests/console/Console/test_properties.py
@@ -1,3 +1,6 @@
+import os
+import sys
+
 from briefcase.console import Console
 
 
@@ -31,3 +34,18 @@ def test_disable():
     console.enabled = False
 
     assert not console.enabled
+
+
+def test_is_interactive_is_tty(console):
+    """Interactivity should match whether a tty is attached to stdout."""
+    assert console.is_interactive is os.isatty(sys.stdout.fileno())
+
+
+def test_is_interactive_non_interactive(non_interactive_console):
+    """Console is non-interactive when stdout has no tty."""
+    assert non_interactive_console.is_interactive is False
+
+
+def test_is_interactive_always_interactive(interactive_console):
+    """Console is interactive when stdout has a tty."""
+    assert interactive_console.is_interactive is True
diff --git a/tests/console/Console/test_release_console_control.py b/tests/console/Console/test_release_console_control.py
index d79c5ee6..df20dc56 100644
--- a/tests/console/Console/test_release_console_control.py
+++ b/tests/console/Console/test_release_console_control.py
@@ -27,12 +27,13 @@ def test_console_is_not_controlled():
     assert console.is_console_controlled is False
 
 
-def test_wait_bar_is_active():
+def test_wait_bar_is_active(interactive_console):
     """An active Wait Bar is stopped."""
-    console = Console()
+    # Always run in interactive mode; otherwise, is_started is will never be True
+    console = interactive_console
 
     with console.wait_bar("Testing..."):
-        # Mock the Wait Bar stop and start methods
+        # Wrap the Wait Bar stop and start methods
         console._wait_bar.stop = Mock(wraps=console._wait_bar.stop)
         console._wait_bar.start = Mock(wraps=console._wait_bar.start)
 
diff --git a/tests/console/Console/test_wait_bar.py b/tests/console/Console/test_wait_bar.py
index abc6f38b..e45edbe8 100644
--- a/tests/console/Console/test_wait_bar.py
+++ b/tests/console/Console/test_wait_bar.py
@@ -115,3 +115,19 @@ def test_wait_bar_keyboard_interrupt_nested(
                 raise KeyboardInterrupt
 
     assert capsys.readouterr().out == output
+
+
+def test_wait_bar_always_interactive(interactive_console):
+    """Wait Bar is not disabled when console is interactive."""
+    with interactive_console.wait_bar():
+        assert interactive_console._wait_bar.disable is False
+        with interactive_console.wait_bar():
+            assert interactive_console._wait_bar.disable is False
+
+
+def test_wait_bar_non_interactive(non_interactive_console):
+    """Wait Bar is disabled when console is non-interactive."""
+    with non_interactive_console.wait_bar():
+        assert non_interactive_console._wait_bar.disable is True
+        with non_interactive_console.wait_bar():
+            assert non_interactive_console._wait_bar.disable is True
diff --git a/tests/console/conftest.py b/tests/console/conftest.py
index 645e9eaa..e197a998 100644
--- a/tests/console/conftest.py
+++ b/tests/console/conftest.py
@@ -1,3 +1,4 @@
+import sys
 from unittest import mock
 
 import pytest
@@ -6,14 +7,26 @@ from briefcase.console import Console
 
 
 @pytest.fixture
-def console():
+def console() -> Console:
     console = Console()
-    console.input = mock.MagicMock()
+    console.input = mock.MagicMock(spec_set=input)
     return console
 
 
 @pytest.fixture
-def disabled_console():
+def disabled_console() -> Console:
     console = Console(enabled=False)
-    console.input = mock.MagicMock()
+    console.input = mock.MagicMock(spec_set=input)
     return console
+
+
+@pytest.fixture
+def non_interactive_console(console, monkeypatch) -> Console:
+    monkeypatch.setattr(sys.stdout, "isatty", lambda: False)
+    yield console
+
+
+@pytest.fixture
+def interactive_console(console, monkeypatch) -> Console:
+    monkeypatch.setattr(sys.stdout, "isatty", lambda: True)
+    yield console