Not all precommit CI unittests are always run

[cpsughrue]

Description:
When a project has a test that unexpectedly fails, the pre-commit CI exits once done running that project's tests instead of going on to run the tests for other projects. Is this intended behavior? If not I'm happy to work on a solution.

An example:
https://buildkite.com/llvm-project/github-pull-requests/builds/42807#018df2b9-0019-4ca1-9116-6cf365c53bb7
On Windows, an MLIR test unexpectedly failed, preventing the LLVM tests from being run

For some context:
I merged #81708 because no LLVM tests failed without realizing that no LLVM tests were run. Now I would like to land #83367 (which fixes the issue with #81708 that wasn't caught until the added LLVM unit test failed on the build bots) but don't feel comfortable doing so until I can at least see the LLVM tests pass on pre-commit CI.

Any comments are greatly appreciated!

[llvmbot]

@llvm/issue-subscribers-infrastructure

Author: Connor Sughrue (cpsughrue)

When a project has a test that unexpectedly fails, the pre-commit CI exits instead of going on to run the tests for other projects. Is this intended behavior?

An example:
https://buildkite.com/llvm-project/github-pull-requests/builds/42807#018df2b9-0019-4ca1-9116-6cf365c53bb7
On Windows, an MLIR test unexpectedly failed preventing the LLVM tests from being run

For some context:
I merged #81708 because no LLVM tests failed without realizing that no LLVM tests were run. Now I would like to land #83367 (which fixes an issue with #81708 that wasn't caught until the added LLVM unit test failed on the build bots) but don't feel comfortable doing so until I can at least see the LLVM tests pass on pre-commit CI.

Any comments are greatly appreciated!

[joker-eph]

Not expected I think...

We run ninja check-clang check-clang-tools check-flang check-lld check-llvm check-mlir check-polly and these are all separate lit invocations, and whenever one fails ninja stops the build.
I am not totally sure how to fix It immediately though.

[cpsughrue]

We run ninja check-clang check-clang-tools check-flang check-lld check-llvm check-mlir check-polly and these are all separate lit invocations, and whenever one fails ninja stops the build. I am not totally sure how to fix It immediately though.

We could separate the commands. For example, attempt to build each target and if the build succeeds run the tests.

ninja llvm && ninja check-llvm
ninja clang && ninja check-clang
...

I'd want to do some tests to make sure this approach does not increase build times but it would guarantee that each project's tests are run if the build succeeds which would be nice!

[joker-eph]

ninja llvm isn't a target, we have ninja test-depends (which isn't fully implemented) for all the test dependency.
In MLIR we have ninja check-mlir-build-only for just building the dependency of the check-mlir target, we should probably standardize this so we could do:

ninja check-clang-build-only check-clang-tools-build-only check-flang-build-only check-lld-build-only check-llvm-build-only check-mlir-build-only check-polly-build-only

Followed by:

ninja  check-clang
ninja  check-clang-tools
ninja  check-flang
ninja  check-lld
ninja  check-llvm
ninja  check-mlir
ninja  check-polly

But here we'd still need the script to run all of these and report a failure if one of them fail, just running them in sequence isn't enough.

We would serialize the test execution (but lit is parallel which mitigates it somehow) but not the build that way.

[tstellar]

It's not clear to me yet why we need this. If we make a change to llvm and an MLIR test fails shouldn't we be investigating and fixing that?

[joker-eph]

It's not clear to me yet why we need this. If we make a change to llvm and an MLIR test fails shouldn't we be investigating and fixing that?

Assume that the MLIR tests is failing at head, and you confirm locally that it fails with and without your change: you still would want the CI to run the LLVM tests right?

[cpsughrue]

It's not clear to me yet why we need this. If we make a change to llvm and an MLIR test fails shouldn't we be investigating and fixing that?

@tstellar I get where you are coming from. At its roots my thought is that during pre-commit CI ninja check-llvm should still be run even if, for example, ninja check-mlir fails.

I think there are many great reasons all the tests should be ran. I created this issue because, to be honest, the llvm-project is large and I don't have the expertise or time to investigate every unrelated test failure. If I can confirm a PR does not cause a particular test to fail it would be nice if all the other tests still ran. I could then make the correct decision on whether to merge a PR without having to wait for someone else to resolve the unrelated failing test. We definitely don't want to create a culture where people start to ignore test failures, so maybe there is a better approach.