Copilot commented on code in PR #54065:
URL: https://github.com/apache/airflow/pull/54065#discussion_r2249918352
##########
airflow-core/tests/unit/cli/conftest.py:
##########
@@ -58,3 +58,80 @@ def parser():
from airflow.cli import cli_parser
return cli_parser.get_parser()
+
+
+class StdoutCaptureManager:
+ """Context manager class for isolating stdout logs from Logger logs."""
+
+ def __init__(self):
+ from io import StringIO
+
+ self._buffer = StringIO()
+ self.original_handlers = []
+ self._final_content = ""
+
+ @property
+ def content(self) -> str:
+ return self._final_content
+
+ def getvalue(self) -> str:
+ return self._final_content
+
+ def splitlines(self) -> list[str]:
+ return self._final_content.splitlines()
+
+ def __enter__(self):
+ import logging
+ from contextlib import redirect_stdout
+
+ # Setup logging isolation
+ root_logger = logging.getLogger()
+ self.original_handlers = list(root_logger.handlers)
+
+ stdout_handlers = [
+ h
+ for h in self.original_handlers
+ if isinstance(h, logging.StreamHandler) and h.stream == sys.stdout
+ ]
+
+ for handler in stdout_handlers:
+ root_logger.removeHandler(handler)
+
+ # Set up redirect_stdout
+ self.stdout_redirect = redirect_stdout(self._buffer)
+ self.stdout_redirect.__enter__()
+
+ return self
+
+ def __exit__(self, exc_type, exc_val, exc_tb):
+ import logging
+
+ # Try to get content before cleanup, but don't fail if we can't
+ try:
+ self._final_content = self._buffer.getvalue()
+ except (ValueError, AttributeError):
+ # Buffer is closed/invalid, but we might have captured content
elsewhere
+ self._final_content = ""
+
+ # Clean up redirect_stdout
+ if hasattr(self, "stdout_redirect"):
+ try:
+ self.stdout_redirect.__exit__(exc_type, exc_val, exc_tb)
+ except:
Review Comment:
Using a bare `except:` clause is discouraged as it catches all exceptions
including system exit and keyboard interrupts. Consider catching specific
exceptions or at minimum use `except Exception:` to avoid catching
BaseException subclasses.
```suggestion
except Exception:
```
##########
airflow-core/tests/unit/cli/commands/test_variable_command.py:
##########
@@ -193,7 +191,7 @@ def test_variables_isolation(self, tmp_path):
assert path1.read_text() == path2.read_text()
def test_variables_import_and_export_with_description(self, tmp_path):
- """Test variables_import with file-description parameted"""
+ """Test variables_import with file-description parameter"""
Review Comment:
The word 'parameted' in the original comment was corrected to 'parameter',
which is the proper spelling.
##########
airflow-core/tests/unit/cli/commands/test_dag_command.py:
##########
@@ -250,22 +250,22 @@ def test_cli_get_dag_details(self):
assert value in out
@conf_vars({("core", "load_examples"): "true"})
- def test_cli_list_dags(self):
+ def test_cli_list_dags(self, stdout_capture):
args = self.parser.parse_args(["dags", "list", "--output", "json"])
- with contextlib.redirect_stdout(StringIO()) as temp_stdout:
+ with stdout_capture as temp_stdout:
dag_command.dag_list_dags(args)
out = temp_stdout.getvalue()
dag_list = json.loads(out)
- for key in ["dag_id", "fileloc", "owners", "is_paused"]:
+ for key in ["dag_id", "fileloc", "owners", "is_paused"]: #
"bundle_name", "bundle_version"?
Review Comment:
The commented code `# "bundle_name", "bundle_version"?` suggests uncertainty
about whether these keys should be included. This comment should either be
resolved by determining if these keys are needed or removed to avoid confusion.
```suggestion
for key in ["dag_id", "fileloc", "owners", "is_paused"]:
```
--
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]
