chore: fix warnings in tests #49
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
=======================================
Coverage 94.63% 94.63%
=======================================
Files 11 11
Lines 205 205
=======================================
Hits 194 194
Misses 11 11 ☔ View full report in Codecov by Sentry. |
| # Prevent unpredictable behavior from import order mismatch | ||
| if get_hooks.__name__ in sys.modules: | ||
| del sys.modules[get_hooks.__name__] |
There was a problem hiding this comment.
Deleting from sys.modules feels a bit less than ideal but for tests I think it's alright. Is this caused by setup_method re-importing get_hooks for each test case?
I'm slightly curious if importing these modules from within the test runner is an alternative fix, but importing anywhere but the top of a file feels so wrong to me so perhaps it's not worth exploring this!
There was a problem hiding this comment.
We could try to import this inside the test, I think I was able to make it work. This type of manipulation can cause python to fail if the wrong modules are deleted. Maybe there is a safer way to do this. 🤔
From what I understand deleting from sys.modules can be used as a way to force module reloading.
This is a dictionary that maps module names to modules which have already been loaded. This can be manipulated to force reloading of modules and other tricks.
There was a problem hiding this comment.
Oh dang! I should've checked the official docs haha. If it's noted that tricks can be done with sys.module, a lot of my concern disappears!
Totally your call on looking for alternatives to this approach! 🚀
There was a problem hiding this comment.
Using run_path instead of run_module fixed our concerns and the warnings
There was a problem hiding this comment.
Incredible find 🙏 Thanks again for looking into this fix and the follow up!
Summary
This PR fixes the warnings in the tests. They originated from miss ordered imported modules, this is due to the way the tests are executed. These changes force python to load the module from scratch.
Testing
Pull this branch, run the tests
Special notes
Requirements
./scripts/install_and_run_tests.shafter making the changes.