Conversation
patricktnast
changed the title
patricktnast
requested review from
albrja,
hussain-jafari,
rmudambi and
stevebachmeier
as code owners
| def _read_metadata(output_dir: Path) -> pd.DataFrame: | ||
| """Read the finished simulation metadata CSV from an output directory.""" | ||
| metadata_path = output_dir / "finished_sim_metadata.csv" | ||
| assert metadata_path.exists(), f"Metadata file not found at {metadata_path}" |
There was a problem hiding this comment.
Nit: This seems unnecessary since we will runtime error on the next line if this fails.
tests/conftest.py
Outdated
| item.add_marker(skip_e2e) | ||
|
|
||
|
|
||
| def is_on_slurm() -> bool: |
There was a problem hiding this comment.
Isn't this a pytest plugin in VTU now?
There was a problem hiding this comment.
it actually is not, but I think it is fair to say that it should be.
tests/conftest.py
Outdated
| return shutil.which("sbatch") is not None | ||
|
|
||
|
|
||
| def is_slow_test_day(slow_test_day: str = SLOW_TEST_DAY) -> bool: |
There was a problem hiding this comment.
This should definitely be a plugin.
| @@ -0,0 +1,22 @@ | |||
| # Minimal vivarium model specification for E2E testing. | |||
| # Runs 2 time steps with 10 simulants -- completes in seconds. | |||
| # No components, no data artifact, no results observers. | |||
There was a problem hiding this comment.
Won't we need observers to do some sorts of E2E tests - e.g. to test that we write results to disk properly?
tests/psimulate/test_e2e.py
Outdated
| for _ in range(10): | ||
| if not results_dir.exists(): | ||
| break # the dir has been removed | ||
| # Take a quick nap to ensure processes are finished with the directory |
There was a problem hiding this comment.
Sometimes copilot comments amuse me
There was a problem hiding this comment.
funny enough, this is actually a direct copy/paste from Easylink
https://github.com/ihmeuw/easylink/blame/main/tests/conftest.py
Not sure where Steve got it from, though!
tests/psimulate/test_e2e.py
Outdated
| self, shared_tmp_path: Path, slurm_project: str | ||
| ) -> None: | ||
| """Expand a completed run by adding draws and seeds, verify new jobs complete.""" | ||
| _, output_dir = _run_basic_simulation(shared_tmp_path, slurm_project) |
There was a problem hiding this comment.
Nit: is it possible to leverage the fact that you have already done a psimulate run in other tests so you don't have to duplicate that work? This is not that important, as these jobs are fast.
tests/psimulate/test_e2e.py
Outdated
| "-P", | ||
| slurm_project, | ||
| "-r", | ||
| "00:10:00", |
There was a problem hiding this comment.
You said above that these sims take a few seconds to run. Can we reduce the run-time request? What about memory?
There was a problem hiding this comment.
Uh, I shouldn't have let copilot keep in the "-- completes in seconds." but they are indeed pretty fast. I can bump it down to a couple minutes to be on the safe side once I add in another component and observer.
There was a problem hiding this comment.
the memory is actually typed in click as int
tests/conftest.py
Outdated
| ) | ||
|
|
||
|
|
||
| def pytest_collection_modifyitems(config: Config, items: list[Function]) -> None: |
There was a problem hiding this comment.
This logic seems to be duplicated across many repos. Is it possible to move it into VTU?
There was a problem hiding this comment.
apparently so!
Co-authored-by: Rajan Mudambi <11376379+rmudambi@users.noreply.github.com>
…m_cluster_tools into pnast/testing/e2e-cluster
| assert deaths_dir.exists() | ||
|
|
||
| deaths_df = pd.read_parquet(deaths_dir) | ||
| assert not deaths_df.empty |
There was a problem hiding this comment.
Check if we have results from each of our parallel runs.
3 participants
Add CLuster-based E2E tests for VCT
Description
testing
Changes and notes
I added some basic tests of psimulate run, restart, and expand. This will help us sort out the jobmon refactor.
I added a load test one too, but it is failing, i think more or less expectedly.
Testing