potiuk commented on code in PR #69324:
URL: https://github.com/apache/airflow/pull/69324#discussion_r3543269853
##########
airflow-core/tests/unit/dag_processing/test_manager.py:
##########
@@ -497,6 +497,21 @@ def
test_terminate_orphan_processes_kills_processor_when_file_is_truly_absent(se
)
processor.kill.assert_called_once_with(signal.SIGKILL)
+ def
test_terminate_orphan_processes_tolerates_stale_file_handle_on_close(self):
+ """A stale NFS file handle on close (e.g. OpenShift) must not crash
the manager."""
+ manager = DagFileProcessorManager(max_runs=1)
+ versioned_file = _get_versioned_file_info("callbacks.py")
+ processor = MagicMock()
Review Comment:
Nit (non-blocking): this test uses a full `MagicMock()` as the processor, so
the production call `processor.close()` hits the mock's stubbed `close` and
never runs the real `DagFileProcessorProcess.close()` — the
`logger_filehandle.close.side_effect = OSError(...)` on the next line is never
reached. It passes because the *old* call site touched `logger_filehandle`
directly, but it doesn't actually exercise the OSError-swallowing logic it
documents; it would still pass if `close()` re-raised. The other two tests use
`self.mock_processor()` (a real `DagFileProcessorProcess` with a mock
filehandle), which *does* exercise the try/except — switching this one to
`self.mock_processor()` would give it real coverage and match the pattern.
--
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]