Parallelized Simulation Tests#3766
Conversation
…rewyx/ci_mode_sim_tests
…rewyx/ci_mode_sim_tests
…ware into Andrewyx/ci_mode_sim_tests
…rewyx/parallel_tests
…rewyx/parallel_tests
There was a problem hiding this comment.
made an actual review below
Why are some tags arguments completely removed, whilst others only remove the "exclusive" portion?
As an example: src/software/ai/hl/stp/tactic/attacker/BUILD:41 has the tags arg fully removed, but src/software/ai/hl/stp/tactic/chip/BUILD:38 only has "exclusive" removed
nycrat
left a comment
There was a problem hiding this comment.
Looks good so far; On my macbook the entire simulated test suite ran in less than 3 minutes! I left a few small comments, and also the tags property should be deleted from all of the BUILD files like Daniel pointed out
| ag_eventually_validation_sequence_set=eventually_validation_sequence_set, | ||
| ag_always_validation_sequence_set=always_validation_sequence_set, | ||
| test_timeout_s=10, | ||
| test_timeout_s=60, |
There was a problem hiding this comment.
60 seems a bit too long compared to other tests
There was a problem hiding this comment.
Its very flaky, I think this + ball placement needs dedicated investigation.
StarrryNight
left a comment
There was a problem hiding this comment.
lgtm! left some comments
williamckha
left a comment
There was a problem hiding this comment.
Makes us wonder how we lived with 1 hr+ long CI times...
Description
Finally runs our simulated tests in parallel! 50 min -> 15 min CI test times! Almost a 400% reduction! Some of the changes here are to either prevent race conditions or to prevent processes from terminating each other.
get_runtime_dir())Testing Done
All tests pass.
Resolved Issues
Resolves #2619
Length Justification and Key Files to Review
Review Checklist
It is the reviewers responsibility to also make sure every item here has been covered
.hfile) should have a javadoc style comment at the start of them. For examples, see the functions defined inthunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.TODO(or similar) statements should either be completed or associated with a github issue