180 Change all multirun testing files to use multirun.jar -DBSetup#187
Conversation
There was a problem hiding this comment.
Pull Request Overview
Switches all testing invocations from singlerun.jar to multirun.jar -DBSetup (and corresponding run commands), adds two YAML config files, and updates the CI workflow to use those configs.
- Integration tests now call
multirun.jarwithtest_create_database.ymlandtest_run.yml - Two new config files (
test_create_database.ymlandtest_run.yml) define DB setup and run parameters - GitHub Actions workflow updated to invoke
multirun.jarwith the new configs
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/simpaths/integrationtest/RunSimPathsIntegrationTest.java | Updated test run invocations to use multirun.jar and YAML configs |
| src/main/java/simpaths/data/Parameters.java | Added detection of missing CSV files to set training flag in tests |
| config/test_run.yml | New config to override default multirun run arguments |
| config/test_create_database.yml | New config to override default DB setup arguments |
| .github/workflows/SimPathsBuild.yml | CI commands updated to use multirun.jar with the new YAML configs |
Comments suppressed due to low confidence (2)
src/main/java/simpaths/data/Parameters.java:3323
- [nitpick] Rename
testListto a more descriptive name likecsvFilesorpopulationCsvFilesto clarify its purpose.
Collection<File> testList = FileUtils.listFiles(new File(Parameters.getInputDirectoryInitialPopulations()), new String[]{"csv"}, false);
src/main/java/simpaths/data/Parameters.java:3323
- Consider adding a unit test for
databaseSetupwhen the input directory is empty to cover the new branch that sets the training flag.
Collection<File> testList = FileUtils.listFiles(new File(Parameters.getInputDirectoryInitialPopulations()), new String[]{"csv"}, false);
| File trainingSchedule = new File("input" + File.separator + "EUROMODoutput" + File.separator + "training" + File.separator + EUROMODpolicyScheduleFilename + ".xlsx"); | ||
| File runSchedule = new File("input" + File.separator + EUROMODpolicyScheduleFilename + ".xlsx"); | ||
| try { | ||
| FileUtils.copyFile(trainingSchedule, runSchedule); |
There was a problem hiding this comment.
Just noticed that this will always copy the policy schedule if training flag is true. So this scenario is possible:
- User runs GUI with only training data
- User modifies policy schedule in GUI to change years
- User runs model
- Model then loads
calculateEUROMODpolicySchedule, tests whether training flag is true, and if so overwrites user's policy file
Ideally, the training policy schedule file should always be loaded automatically at the very start if only training data is available, and this step should only occur before the user specifies the file.
Or, this file never gets used and if the policy schedule file doesn't match it checks and errors out?
|
Looks good to me |
What
singlerun.jar -Setupcalls in testing files tomultirun.jar -DBSetupcallstest_create_database.ymltest_run.ymlWhy
SimPathsMultiRun: point towards (and test) consistency in usingmultirun.jarto both setup and run SimPaths