Test workflow without cache keys#131
Conversation
|
Leaning towards not including the no-cache workflow. If this fails without cache we'll know from the failed CI run, so skipping it to avoid the 20+min CI runs. |
|
Changed my mind. We can skip the actual tests, and the check to make sure the images build without cache should take roughly the same amount of time. And this only happens to changes in this repo. If it's annoying we can remove it later, but for now it makes me feel a little better to know we are testing this code path. |
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
Thanks! Some nits, nothing blocking :)
|
|
||
| - name: Configure AWS credentials for S3 build cache | ||
| if: ${{ inputs.s3-access-key-id }} != "" && ${{ inputs.s3-secret-access-key }} != "" | ||
| if: inputs.s3-access-key-id != '' && inputs.s3-secret-access-key != '' |
There was a problem hiding this comment.
GitHub actions syntax is so subtle it is really annoying. Maybe I should pick up my idea of writing a linter for this again ...
My first guess at what is wrong here would have been:
| if: inputs.s3-access-key-id != '' && inputs.s3-secret-access-key != '' | |
| if: ${{ inputs.s3-access-key-id != '' }} && ${{ inputs.s3-secret-access-key != '' }} |
| - uses: ./.github/actions/run-interop-ping-test | ||
| with: | ||
| # It's okay to not run the tests, we only care to check if the tests build without cache. | ||
| test-filter: "no test matches this, skip all" |
| @@ -0,0 +1,19 @@ | |||
| on: | |||
| workflow_dispatch: | |||
There was a problem hiding this comment.
Do we need this?
| workflow_dispatch: |
| pull_request: | ||
| push: | ||
| branches: | ||
| - "master" |
There was a problem hiding this comment.
We share the same triggers with the other workflow right? Why not have it in the other file as a separate job?
There was a problem hiding this comment.
I wanted it to run in parallel and isolated. Is that the case with a separate job?
There was a problem hiding this comment.
Yes jobs run in parallel in a workflow unless you declare a dependency with needs:
| docker buildx build \ | ||
| --load \ | ||
| -t $IMAGE_NAME "$@" |
There was a problem hiding this comment.
Would it make sense to do something like:
| docker buildx build \ | |
| --load \ | |
| -t $IMAGE_NAME "$@" | |
| docker buildx build \ | |
| --load \ | |
| $CACHING_OPTIONS \ | |
| -t $IMAGE_NAME "$@" |
Where $CACHING_OPTIONS is conditionally defined instead of the entire command?
MarcoPolo
force-pushed
the
marco/no-keys
branch
from
eb381b6 to
9e87b1f
Compare
) * Update Go test implementations to match new spec * Update JS test implementation * Update Rust test Implementation * Update root Makefile * Update runner to new spec * Use composite action and S3 caching * Not using GHA cache anymore * Try removing access key from env * Test workflow without cache keys (#131) * Test if it works without s3 cache keys * Fix if statement * Fix if statement * Always use buildkit * Undo debug change * Add no cache workflow * Skip test in no-cache workflow * Update .github/workflows/no-cache-multidim-interop.yml * Same workflow; use CACHING_OPTIONS * Add Browser WebRTC test (#130) * Add webrtc to JS test * Add onlyDial to all queries * Update versions.ts * Remove unneeded timeout overrides
…ibp2p#121) * Update Go test implementations to match new spec * Update JS test implementation * Update Rust test Implementation * Update root Makefile * Update runner to new spec * Use composite action and S3 caching * Not using GHA cache anymore * Try removing access key from env * Test workflow without cache keys (libp2p#131) * Test if it works without s3 cache keys * Fix if statement * Fix if statement * Always use buildkit * Undo debug change * Add no cache workflow * Skip test in no-cache workflow * Update .github/workflows/no-cache-multidim-interop.yml * Same workflow; use CACHING_OPTIONS * Add Browser WebRTC test (libp2p#130) * Add webrtc to JS test * Add onlyDial to all queries * Update versions.ts * Remove unneeded timeout overrides
2 participants
Might make sense to have this as a permanent workflow so that we make sure this doesn't break.
Draft for now as I fiddle with CI