Skip to content
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

Allow test failures for build uploads when not on release branch or tags #420

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ericphanson
Copy link

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.

@DilumAluthge
Copy link
Member

We do want to gate it for releases. Can we make the behavior different for nightlies vs releases?

@ericphanson
Copy link
Author

ah I see. I am having trouble figuring out how to do that. I think we want the conditional (build.branch =~ /^release-/) || (build.tag =~ /^v/) or similar somewhere, but I can't figure out how to get a conditional dependency, or a conditional allowed-failure... 🤔

@ericphanson
Copy link
Author

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 allow_failures

@ericphanson ericphanson changed the title Upload after build without waiting for tests Allow test failures for build uploads when not on release branch or tags Jan 25, 2025
@staticfloat
Copy link
Member

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?

@IanButterworth
Copy link
Member

Sometimes there are PRs that don't yet have all the tests figured out, but would be good for people to try out. The --trim PR was one of those.

@giordano
Copy link
Member

I don't think there's much use uploading these to S3, is there?

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

@ericphanson
Copy link
Author

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.

@staticfloat
Copy link
Member

Fair enough, the juliaup feature makes this much more useful.

@DilumAluthge
Copy link
Member

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.

@DilumAluthge
Copy link
Member

Also needs treehash re-signing.

@DilumAluthge
Copy link
Member

fatal: pipeline interpolation of "upload_linux.yml" failed: interpolating remaining fields: $ALLOW_TEST_FAILURES: not set

@ericphanson
Copy link
Author

looks like I missed a codepath for the allowed-to-fail upload jobs, should be covered now

@DilumAluthge
Copy link
Member

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).

@ericphanson
Copy link
Author

added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants