Unit tests for staging datasets #258
Conversation
Codecov Report
@@ Coverage Diff @@
## main #258 +/- ##
==========================================
+ Coverage 85.07% 91.61% +6.53%
==========================================
Files 29 33 +4
Lines 1682 1919 +237
==========================================
+ Hits 1431 1758 +327
+ Misses 251 161 -90
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -11,6 +11,10 @@ | |||
| from .mp import cpu_count, create_process_pool_executor | |||
|
|
|||
|
|
|||
| def _get_n_workers() -> int: | |||
There was a problem hiding this comment.
Required for testing. In GHA, running with more than 1 worker causes the test to fail.
There was a problem hiding this comment.
Can you point me at more information on this constraint? I'm trying to understand why it would fail on GHA.
If it is the case that GHA processes may not fork more than some limited number of child processes, we would be better off just testing this with the command line arg (args param) with a --max-workers 1. This is the only code that calls create_process_pool_executor with the optional override, and it only affects the behavior when args lacks a max_workers value.
This all seems cleaner / simpler than a mock, at least IMHO :-)
There was a problem hiding this comment.
The error is here:
concurrent.futures.process.BrokenProcessPool: A process in the process pool was terminated abruptly while the future was running or pending.
I didn't notice that the max_workers can be specified in args. Will use that. I'm not sure it's worth investigating the issue above - in my experience testing doesn't always play well with multiprocessing, so since this test isn't specifically meant to test multiprocessing, I think we can just disable it.
There was a problem hiding this comment.
Ah, this means something killed the child process. Commonly this is an out-of-memory condition on the system, or something else that causes an uncontrolled failure (segv, etc). Super hard to diagnose in that environment.
Add unit tests for these 3 functions:
source_assetsload_manifests