mach: Add --update-expectations flag to test-wpt - #45521
Merged
Merged
Conversation
TimvdLippe
force-pushed
the
add-update-expectations-flag
branch
from
June 9, 2026 19:22
52ab26d to
6738820
Compare
Member
|
Might be nice to have a book PR for this as well! |
TimvdLippe
force-pushed
the
add-update-expectations-flag
branch
from
June 9, 2026 19:25
6738820 to
fecbab2
Compare
Contributor
Author
Yup will do once this merges |
TimvdLippe
force-pushed
the
add-update-expectations-flag
branch
from
June 9, 2026 19:27
fecbab2 to
881535d
Compare
With this new flag, it automatically runs `update-wpt` at the end, without the need to specify a `--log-raw` argument. Fixes servo#45476 Testing: Local runs with the various combinations of flags Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
TimvdLippe
force-pushed
the
add-update-expectations-flag
branch
from
June 9, 2026 19:32
881535d to
cc52be0
Compare
Loirooriol
reviewed
Jun 9, 2026
| def test_wpt(self, servo_binary: str, multiprocess: bool, update_expectations: bool, **kwargs: Any) -> int: | ||
| if update_expectations: | ||
| if kwargs["log_raw"]: | ||
| print("Do not specify --log-raw when updating tests directly") |
Contributor
There was a problem hiding this comment.
While it seems a bit weird to use both, would there really be a problem?
Contributor
Author
There was a problem hiding this comment.
Yes, it would break second time parsing. Initially I allowed for both, but then paraing it for update would fail since it expects a string.
nicoburns
reviewed
Jun 9, 2026
| @CommandArgument("--multiprocess", "-M", default=False, action="store_true", help="Run in multiprocess mode") | ||
| @CommandArgument( | ||
| "--update-expectations", | ||
| "-U", |
Contributor
There was a problem hiding this comment.
Any chance of lowercase -u? Partly because uppercase short flags conventionally mean NOT. Partly because almost all the git commands have a really useful flag under -u and it would match my general pattern of "that useful option I use all the time is -u" (not the most rational justification I admit)
Taym95
approved these changes
Jun 9, 2026
Taym95
left a comment
Member
There was a problem hiding this comment.
LGTM ✅ , really helpful thanks!
shubhamg13
approved these changes
Jun 10, 2026
Signed-off-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
TimvdLippe
enabled auto-merge
June 10, 2026 04:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With this new flag, it automatically runs
update-wptat the end, without the need to specify a--log-rawargument.Fixes #45476
Testing: Local runs with the various combinations of flags