Skip to content

Run pre-merge build with -k 0 to ensure all tests runs #84828

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

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

joker-eph
Copy link
Collaborator

The -k option allows to continue the build after failures as much as possible. This is useful here because when we run

ninja check-llvm check-clang

we would like the clang tests to run even if there is a failure in a llvm tests.

Fixes #83371

The -k option allows to continue the build after failures as much
as possible. This is useful here because when we run

> ninja check-llvm check-clang

we would like the clang tests to run even if there is a failure in
a llvm tests.

Fixes llvm#83371
@joker-eph joker-eph requested review from tstellar and cpsughrue March 11, 2024 20:45
Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good solution. How does this behave if a file in lib/Support fails to build, will it still try to run the all the tests?

@joker-eph
Copy link
Collaborator Author

It will continue to build all the targets it can build: if there is a test target that can run without libSupport it will get there eventually. But it can't build anything that depends on libSupport.

Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, makes sense.

Copy link
Contributor

@cpsughrue cpsughrue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@joker-eph joker-eph merged commit 65fd664 into llvm:main Mar 11, 2024
@joker-eph joker-eph deleted the fix-premerge branch March 11, 2024 21:00
@dsandersllvm
Copy link
Collaborator

This seems to have broken the pre-commit CI because the -k 0 has been inserted between the -C and it's directory argument. See #84842

cpsughrue added a commit that referenced this pull request Mar 11, 2024
#84828 added `-k 0` to pre-merge CI so that if one job fails the others
would continue building. This pull request fixes the location of `-k 0`
in the ninja command line.

Resolves #84842 and #83371
@joker-eph
Copy link
Collaborator Author

I was satisfied with the green mark for the buildkite test passing and didn't check the log: it didn't run anything...

This seems like an issue with our premerge config: it does not run at all when we modify this script!

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

Successfully merging this pull request may close these issues.

Not all precommit CI unittests are always run
4 participants