Dev-iL commented on code in PR #59785:
URL: https://github.com/apache/airflow/pull/59785#discussion_r2649015859
##########
airflow-core/tests/unit/cli/commands/test_dag_command.py:
##########
@@ -225,7 +225,7 @@ def test_cli_report(self, stdout_capture):
dag_command.dag_report(args)
out = temp_stdout.getvalue()
- assert "airflow/example_dags/example_complex.py" in out
Review Comment:
Claude says:
Since the output is formatted by `AirflowConsole().print_as()` with JSON
output, the file path would appear as a JSON string value. A more robust test
would be:
### Option 1: Check JSON contains the filename (current approach, acceptable)
```python
assert "example_complex.py" in out
```
### Option 2: More precise - check it ends a JSON string value
```python
assert 'example_complex.py"' in out # includes trailing quote
```
### Option 3: Parse JSON and check properly (most robust but more complex)
```python
import json
data = json.loads(out)
assert any(item["file"].endswith("example_complex.py") for item in data)
```
My recommendation: The current `in` check is pragmatically fine for this
test's purpose (verifying the CLI works). If you want to be slightly more
precise without over-engineering, you could use:
```python
assert 'example_complex.py"' in out
```
This ensures it's matching a filename at the end of a JSON string value, not
arbitrary text. But honestly, the simple `in` check is unlikely to cause false
positives in practice, and keeping the test simple has value too.
--
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]
