-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[CI] Run Linux premerge CI on libc++ changes. #155188
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
base: main
Are you sure you want to change the base?
Conversation
This is intended to catch failures like https://lab.llvm.org/staging/#/builders/192/builds/1671 (LLDB formatter related) before they end up landing in tree.
@ldionne @philnik777 @EricWF Any thoughts on this? |
I thought the lldb tests should already be run in the bootstrapping build? |
It doesn't look like it. #94379 landed with all CI passing despite there being an LLDB failure that the premerge bots then caught. https://lab.llvm.org/staging/#/builders/192/builds/4005 We can do it either way, but I think having the coverage here is important, ideally to coordinate patches between LLDB and libc++. |
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.
Hmmm libc++ folks had an issue with this back when we discussed running LLDB tests on libc++ CI. Mainly because of how long they might take to run. That's why we only run the data-formatter tests in the libc++ CI (#88312). But maybe that changed (or maybe I'm misunderstanding this current PR)
Oh I see @philnik777 already chimed in. I think i see what happened. The test that failed was skipped on Linux for the longest time. I re-enabled it very recently specifically to catch libc++<->LLDB breakages: #157649 But the pre-merge CI on the libc++ PR probably ran without being rebased on that change. So the test was skipped. I think at this point all the libc++-specific tests in LLDB are run as part of the bootstrapping build |
Github actions will run the changes as if they were merged into If this is running as part of the bootstrapping build, that seems reasonable enough to me. I just want to make sure we aren't missing test coverage for these sorts of changes. This is the second or third time the lldb data formatters for libc++ have broken all of premerge for an extended length of time. |
Agreed
Pretty sure if we tried relanding #94379, the pre-merge CI would fail on it (would be good to confirm :)) |
This is intended to catch failures like
https://lab.llvm.org/staging/#/builders/192/builds/1671 (LLDB formatter related) before they end up landing in tree.