-
Notifications
You must be signed in to change notification settings - Fork 309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run tests against nightly CLI bundles #600
Conversation
.github/workflows/pr-checks.yml
Outdated
VERSIONS_JSON='[null, "latest"]' | ||
fi | ||
# Use both `tools: null` and `tools: (nightly URL)` in the integration tests. | ||
VERSIONS_JSON='[null, "${{ steps.get-url.outputs.nightly-url }}"]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to still be testing against latest
because it exercises the hardcoded default configured within the Action repo, and sometimes may be different from both null and nightly. Can we matrix over all three?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, done!
4e5b69e
to
4907bc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! Hopefully won't make too many matrixed jobs, otherwise we might have to split up the workflow. Merge after the nightly bundle is published.
.github/workflows/pr-checks.yml
Outdated
# Just use `tools: null` to avoid duplication in the integration tests. | ||
VERSIONS_JSON='[null]' | ||
# Skip `tools: latest` since it would be the same as `tools: null` | ||
VERSIONS_JSON='[null, "${{ steps.get-url.outputs.nightly-url }}"]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: You may want to pull this one into an env var too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
4907bc2
to
64b73fd
Compare
There's actually already a nightly release up from the workflow's testing, but the PR check is failing because https://github.com/github/semmle-code/pull/39627 isn't merged yet I think. We'll have to wait for the next nightly build after that's merged before this can go in 🙂 |
64b73fd
to
5f01591
Compare
I'm a little bit confused. This looks like it only runs the |
The way these tests are setup is a little weird. We use the |
5f01591
to
9547001
Compare
This PR changes the integration tests of the CodeQL Action to also run against a recent nightly build of the CodeQL bundle. This should help catch compatibility issues arising from changes in the Action earlier.