[WIP] Improvements to speed CI and testing #159
Conversation
|
I think there are a few tests that cannot run in parallel because they clobber each other (namely, the PDDLStream and system integration tests). I wonder if there are pytest decorators that can be thrown in there to force them to run not in parallel? Weirdly enough, even though there are test failures the CI shows up as green lol... |
Yeah playing around with pytest-parallel vs pytest-xdist, but now I see kevlened/pytest-parallel#104. Will try xdist in moment. Will also see if we can at least cache pip deps for pytests... I'm ignoring docker build caches for the moment. |
The biggest slowness in tests is the PDDLStream installation... so bang for buck wise, caching the Docker builds is gonna be it. But I also don't know how much is possible on free/public repos. |
This reverts commit 2c18002.
You are definitely not wrong... on the plus side caching pip dependencies might save 1 or 2 seconds. You don't happen to have a dockerhub account do you? I think the free tier would probably cover what you need, since github registry charges for storage.
Yeah that confused me. Something else is amok with CI here since I reverted that change and broke a unit test and... https://github.com/sea-bass/pyrobosim/actions/runs/7803256591/job/21282622248?pr=159#step:7:53 |
| SUCCESS=0 | ||
|
|
||
| # Do not swallow errors with tee | ||
| set -o pipefail |
There was a problem hiding this comment.
Since you're piping to tee below I think you need this: https://unix.stackexchange.com/q/14270
So that:
https://github.com/sea-bass/pyrobosim/pull/159/checks#step:7:130
| @@ -11,6 +11,12 @@ | |||
| SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | |||
| TEST_RESULTS_DIR="${SCRIPT_DIR}/results" | |||
|
|
|||
| # Ensure we run everything for coverage purposes, but ensure failures are returned by the script | |||
| SUCCESS=0 | |||
There was a problem hiding this comment.
We could alternatively just set -e or something, but did this in case you want ros coverage to run even when the pytests fail.
|
Closing in favor of: #160 since this doesn't need to be on my fork anymore |
No description provided.