Make tests run under the 256 tests limit; give this repo the ❤️ it deserves. #778
Conversation
vjovanov
changed the title
vjovanov
force-pushed
the
vj/gh-681-test-matrix
branch
13 times, most recently
from
993c827 to
7f57c81
Compare
vjovanov
changed the title
vjovanov
force-pushed
the
vj/gh-681-test-matrix
branch
8 times, most recently
from
e6b41c7 to
f2afabe
Compare
vjovanov
changed the title
vjovanov
force-pushed
the
vj/gh-681-test-matrix
branch
from
f4bc376 to
03c0189
Compare
vjovanov
added
the
high-priority
jormundur00
left a comment
jormundur00
left a comment
There was a problem hiding this comment.
Great work! Left a few comments, but all in all LGTM.
| test-changed-infrastructure: | ||
| name: "🧪 ${{ matrix.coordinates }} (GraalVM for JDK ${{ matrix.version }} @ ${{ matrix.os }})" | ||
| if: needs.get-changed-infrastructure.result == 'success' && needs.get-changed-infrastructure.outputs.none-found != 'true' && github.repository == 'oracle/graalvm-reachability-metadata' |
There was a problem hiding this comment.
I don't think we need to check github.repository == 'oracle/graalvm-reachability-metadata' here, as if get-changed-infrastructure is skipped test-changed-infrastructure will also be skipped due to needs.get-changed-infrastructure.
| name: "🧪 All build-logic triggered tests have passed" | ||
| runs-on: "ubuntu-22.04" | ||
| timeout-minutes: 1 | ||
| if: ${{ always() }} && github.repository == 'oracle/graalvm-reachability-metadata' |
There was a problem hiding this comment.
Don't think we need the repository check here either, as it's already made if the needed prerequisite job is ran.
| - '**.md' | ||
| - 'library-and-framework-list*.json' | ||
| paths: | ||
| - 'metadata/*' |
There was a problem hiding this comment.
Are we sure we want to check metadata/* and not metadata/** here? Not an expert on wokflow syntax, but as I understand the wildcard definitions here, * should only include direct metadata directory descendants, while metadata/** should include nested directories too.
If that's correct, we should probably change that in other places where /* is used.
|
Since this is blocking us I will merge the PR and address the comments in the follow up PR. |
vjovanov
force-pushed
the
vj/gh-681-test-matrix
branch
from
42b1a57 to
b9af88c
Compare
- All commands now support a -Pcoordinates=k/n mode for batched execution. - One stop shop testing command: ./gradlew testAllParallel -Pparallelism=n - Documentation and structure cleanup (docs). - CLine support. - Complete command cleanup and simplification.
vjovanov
force-pushed
the
vj/gh-681-test-matrix
branch
from
b9af88c to
aaf3d18
Compare
melix
left a comment
melix
left a comment
There was a problem hiding this comment.
This is a very large PR and tbh I'm not sure I can validate the changes, it's not trivial.
| - name: "Install required tools" | ||
| run: | | ||
| curl -sSfL https://raw.githubusercontent.com/anchore/grype/main/install.sh | sudo sh -s -- -b /usr/local/bin | ||
| curl -sSfL https://get.anchore.io/grype/v0.104.0/install.sh | sudo sh -s -- -b /usr/local/bin |
There was a problem hiding this comment.
Not different from the previous implementation, but running arbitrary shell commands from remote makes me kind of nervous. Maybe we should at least have a checksum to ensure the remote repo hasn't been compromised.
3 participants
ci.jsonwhere we store the matrix of JDK and OS and flags. If this file is changed, run all tests in batches.test-changed-metadata.ymlshould just check if metadata changes.test-changed-infrastructure.ymlshould test only a representative set of libraries.-Pcoordinates=k/nmode for batched execution../gradlew testAllParallel -Pparallelism=ndocs/DEVELOPING.mddocs/CI.mdAGENTS.mdFixes: #681