Upload PR builds even when tests fail#420
Conversation
|
We do want to gate it for releases. Can we make the behavior different for nightlies vs releases? |
|
ah I see. I am having trouble figuring out how to do that. I think we want the conditional |
|
I gave it a try, but I don't know if it will work -- in particular, I don't know if ENV interpolation works like this for |
|
If the concern is that you want to be able to download a build of Julia that is failing tests so you can inspect it, you can download it from the buildkite interface in the Artifacts tab. I don't think there's much use uploading these to S3, is there? |
|
Sometimes there are PRs that don't yet have all the tests figured out, but would be good for people to try out. The |
My guess is that this would be useful for juliaup? I'm not sure where it downloads the tarball from, but if it's S3 that'd be a discriminant here |
|
Yeah, my gripe here is I wanted to use the Pkg apps feature before JuliaLang/julia#57187 was merged, but I couldn't use the juliaup PR feature for it since the macos CI had failed. It was confusing initially as well bc juliaup reports it can't be installed, but I saw the PR is there and didn't understand that uploads were gated on CI passing. |
|
Fair enough, the juliaup feature makes this much more useful. |
|
I don't think this is ready to merge yet? We need to make it so that release branches and tags DO gate on tests, but nightlies and PRs do NOT gate on tests. It looks like this PR attempts to implement that, but something about the implementation is causing CI to fail. |
|
Also needs treehash re-signing. |
|
|
looks like I missed a codepath for the allowed-to-fail upload jobs, should be covered now |
|
Before we re-sign the pipelines, could you add comments that explain the purpose of the logic (i.e. that we want to gate on the tests in release branches and tags, but we don't want to gate on the tests in master/nightlies/prs). |
|
added |
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the CI/CD pipeline to allow test failures during build uploads for non-release branches and non-version tags, enabling nightly and PR builds to be uploaded to S3 even when tests fail.
- Updates upload pipeline steps to conditionally allow test failures based on branch/tag type
- Adds logic to set
ALLOW_TEST_FAILURESenvironment variable based on whether the build is on a release branch or version tag - Modifies all platform upload configurations (Windows, macOS, Linux, FreeBSD) to respect the new test failure policy
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pipelines/main/launch_upload_jobs.yml | Adds conditional logic to allow test failures for non-release builds |
| pipelines/main/platforms/upload_*.yml | Updates dependency configuration to allow test step failures when permitted |
| *.signature files | Updated cryptographic signatures for modified pipeline files |
|
@ericphanson @DilumAluthge lets merge this, if you agree it's still in good shape? |
DilumAluthge
left a comment
There was a problem hiding this comment.
Two requests:
- Rename variable name to make it more clear that this is specifically just affecting whether or not we upload - it doesn't affect anything else.
- Ideally, I'd like to have a small GitHub Actions CI job that runs a Bash script to just test that our Bash syntax for pattern matching is correct, and that it gives the expected output when the branch is
release-somethingor the tag isvSOMETHING.
Once those two are done, this is good to merge from my POV.
Revert "Upload after build without waiting for tests" This reverts commit 9c8fd9e. try to do conditional allowed failures add `ALLOW_TEST_FAILURES` in second place add comments
…ogic - Rename ALLOW_TEST_FAILURES to UPLOAD_EVEN_IF_TEST_FAILURES for clarity to avoid confusion with Buildkite's 'allow fail' soft-fail mechanism - Add main/master branch checks to ensure uploads don't happen on test failures for the main branch (nightlies) - Ensure correct behavior: * Tags (v*): Don't upload if tests fail * Release branches: Don't upload if tests fail * Main/master branch: Don't upload if tests fail * PR builds: Upload even if tests fail - Add GitHub Actions workflow to test the upload logic Addresses: JuliaCI#420 (review) Co-authored-by: Claude <noreply@anthropic.com>
1ed1a42 to
6309270
Compare
I propose this change because currently if tests fail, the build does not upload. I think some of the point of using nightly/pr-builds is to test things manually, and potentially to debug failing tests and issues like that. Therefore, I don't think uploads should be gated on tests passing.