-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CI] Test lit when it is changed #159359
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
[CI] Test lit when it is changed #159359
Conversation
This patch makes it so that we run the lit tests when lit is changed. This can make it much easier to catch portability issues with the tests (like tests not working properly on Windows). It's also helpful as we spend time working on enabling the internal shell by default.
✅ With the latest revision this PR passed the Python code formatter. |
) | ||
self.assertEqual( | ||
env_variables["project_check_targets"], | ||
"check-bolt check-clang check-clang-tools check-flang check-lit check-lld check-lldb check-llvm check-mlir check-polly", |
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'm kind of confused by how we arrive at this list given the changes you made... Is there some kind of fallback for "test everything" involved here?
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.
Given touching lit (llvm/utils/lit
) also touches llvm, we end up testing everything that touching LLVM would. I think this is reasonable enough behavior given it serves as integration testing for lit.
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.
Oh, that makes sense. Thanks!
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.
Why can't the list just be "check" - doesn't that check every enabled project?
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 think that would be check-all
(maybe synonymous with check
?). We want to have more control over which test suites we run than check-all
gives us (eg build projects but don't test them, split out the runtimes, test runtimes multiple times).
This patch makes it so that we run the lit tests when lit is changed. This can make it much easier to catch portability issues with the tests (like tests not working properly on Windows). It's also helpful as we spend time working on enabling the internal shell by default.