rjgoyln commented on code in PR #72707:
URL: https://github.com/apache/airflow/pull/72707#discussion_r3970437489
##########
airflow-core/tests/unit/cli/commands/test_info_command.py:
##########
@@ -162,6 +162,15 @@ def test_show_info_anonymize(self, stdout_capture):
assert airflow_version in output
assert "postgresql+psycopg2://p...s:PASSWORD@postgres/airflow" in
output
+ @mock.patch.dict(os.environ, {"FORCE_COLOR": "1"})
Review Comment:
`FORCE_COLOR=1` alone doesn't guarantee a color system:
`_detect_color_system()` returns `None` when `TERM` is `dumb`/`unknown`, so
this passes even with `color_system=None` reverted. Measured with
`color_system="auto"` - `TERM` unset gives `standard` (escapes present),
`TERM=dumb` gives `None` (no escapes). Pinning the terminal type makes it
deterministic:
```suggestion
@mock.patch.dict(os.environ, {"FORCE_COLOR": "1", "TERM":
"xterm-256color"})
```
##########
airflow-core/tests/unit/cli/commands/test_info_command.py:
##########
@@ -162,6 +162,15 @@ def test_show_info_anonymize(self, stdout_capture):
assert airflow_version in output
assert "postgresql+psycopg2://p...s:PASSWORD@postgres/airflow" in
output
+ @mock.patch.dict(os.environ, {"FORCE_COLOR": "1"})
+ def test_render_text_stays_plain_on_a_color_terminal(self):
Review Comment:
First test here to go through the full `show()` path, so it initialises the
`ProvidersManager` singleton. The sibling `--file-io` test takes
`cleanup_providers_manager` for that reason - without it the initialised
provider configuration leaks into whatever runs next on the same worker.
```suggestion
def test_render_text_stays_plain_on_a_color_terminal(self,
cleanup_providers_manager):
```
##########
airflow-core/src/airflow/cli/commands/info_command.py:
##########
@@ -335,10 +335,11 @@ def show(self, output: str, console: AirflowConsole |
None = None) -> None:
def render_text(self, output: str) -> str:
"""Export the info to string."""
- console = AirflowConsole(record=True)
- with console.capture():
+ # No color_system: the captured text is uploaded as a file, so escape
codes are noise.
+ console = AirflowConsole(color_system=None)
Review Comment:
Optional, same reasoning as `color_system`: `_width` is pinned to 200 only
when not a tty, so on a real terminal the uploaded report is laid out at the
terminal's width - at 80 columns the long `system_path` / `python_path` values
wrap across the table borders. `width=200` would make the uploaded file
independent of where it was generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
