KevinGG commented on pull request #13881:
URL: https://github.com/apache/beam/pull/13881#issuecomment-772102411


   If `cleanup` is invoked more than once, the extra invocation must have come 
from the `ie.new_env()` when the current environment instance is not None. The 
old code patches `cleanup` with the decorator. It seems that when running tests 
on Jenkins, sometimes, even setting `ie._interactive_beam_env = None` 
forcefully wouldn't guarantee the module property is `None` when the subsequent 
statements in the same test method are executed.
   
   In that case, the test should be resilient to global state of 
`ie._interactive_beam_env`. By moving patches inside each test function after 
`ie.new_env()`, the test mock is not affected by whether `ie.new_env()` 
invoking `cleanup` or not. And the assertions can focus on testing the logic 
within each test.
   
   Couldn't reproduce the flakiness locally, but tested with 4 CPU cores 
running concurrently for 100 flake runs:
   ```
   pytest --flake-finder --flake-runs=100 -n 4 
apache_beam/runners/interactive/interactive_environment_test.py
   ```
   ![Screen Shot 2021-02-02 at 4 06 22 
PM](https://user-images.githubusercontent.com/4423149/106678966-9a09ad00-6570-11eb-94d9-e868d387c1b7.png)
   


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to