Closed kyleam closed 9 months ago
This is ready to review but should be merged after its base (gh-307).
The integration tests for this branch took about 20 minutes. The base run took around 12 minutes, which makes me worried this is adding a bit of time. However...
I don't expect this to add much time because 1) it's just adding one local test and 2) it only adds ~5 seconds on my end.
other runs without this commit took longer than the base's run (~14-16 minutes)
https://github-drone.metrumrg.com/metrumresearchgroup/bbi/1037/2/6 https://github-drone.metrumrg.com/metrumresearchgroup/bbi/1036/2/6 https://github-drone.metrumrg.com/metrumresearchgroup/bbi/1039/2/6 https://github-drone.metrumrg.com/metrumresearchgroup/bbi/1038/2/6
a re-push of the same tree as this PR took ~16 minutes
https://github-drone.metrumrg.com/metrumresearchgroup/bbi/1043/2/6
So my current guess is that the timing variability above is due to something other than this PR's additional scenario (e.g., the runner being primed for SGE).
I was thinking the same thing [...]
Thanks for your input here (and good point about the difference between the companion builds).
As of aa686d3 (tests: removing more scenarios from tests that don't need so many divergent paths [...], 2021-09-23), the period_test scenario is no longer used by any tests. This scenario provides a regression test for a fix in v2.3.1, so it's worth keeping around.
Restore its use in TestBbiCompletesLocalExecution.