jason810496 commented on PR #45631:
URL: https://github.com/apache/airflow/pull/45631#issuecomment-2599741396

   > Update:
   > Luckily, I found that it was caused by the tests/ti_deps/ module after a 
few iterations!
   > `tests/ti_deps/deps/test_ready_to_reschedule_dep.py`
   
   TL;DR:  
   The global state in the `executor_loader` module can cause side effects in 
other test cases if not properly torn down.  
   
   The issue is caused by the `executor_loader` module, as it stores some state 
in global variables:  
   
https://github.com/apache/airflow/blob/main/airflow/executors/executor_loader.py#L48-L54
  
   This can lead to errors in `Executor.load_executor` or 
`Executor.get_default_executor` , as these methods rely on looking up 
`executor_name` in global variables. If a test interacts with `executor_loader` 
and doesn't clear the global variables, it may cause side effects in subsequent 
tests.  
   
   ---
   
   In other test cases that use the `executor_loader` module, `reload` is 
employed.
   However, in the current test case, we need to mock the 
`Executor.load_executor` and `Executor.get_default_executor` methods. Mixing 
`reload(executor_loader)` with mocks results in the mock object never being 
called, as the callable used during the test runtime bypasses the mock.  
   
   From my perspective, cleaning the global variables in `executor_loader` 
should suffice for tearing down the state. I will create another PR to 
investigate whether this approach is correct and to evaluate how much test 
speed can be improved by replacing all `reload(executor_loader)` calls with 
`clean_executor_loader()`.  
   


-- 
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]

Reply via email to