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

Flip --experimental_use_validation_aspect #23637

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Sep 17, 2024

RELNOTES: Validation actions now run concurrently with tests.

@fmeum fmeum requested a review from lberki September 17, 2024 07:03
@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 17, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 17, 2024

@lberki I learned that this flag is essentially always enabled at Google and think that it would be the more useful default for Bazel as well (with #23589 fixed).

@fmeum
Copy link
Collaborator Author

fmeum commented Sep 17, 2024

Okay, there are many more BES tests checking exact counts than I imagined. I can fix them, but this leads to the question whether we would want to exclude the ValidateTarget aspect from the BES or represent it differently. @lberki What do you think? How are you handling this at Google?

@lberki
Copy link
Contributor

lberki commented Sep 17, 2024

I'm afraid I must disappoint you here -- the way our internal integration test suite works is that the flags in the repository-level bazelrc do not track their production values closely. Every once in a while, we get the idea to synchronize them, but there are good reasons for them to diverge so it never comes to anything :(

It's actually even worse than that because we only use --experimental_use_validation_aspect in most of our builds: some systems built around Blaze got confused by the very extra BES messages these tests complain about and we haven't get gotten around fixing them...

My ideal course of action would be to flip the flag and update the tests nevertheless, thereby moving the needle towards this flag being on. I'd totally tell how how many tests we have internally that break, but I'm afraid I'm somewhat oversubscribed today so I can't...

@iancha1992 iancha1992 added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Sep 17, 2024
RELNOTES: Validation actions now run concurrently with tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants