Add et query tests and et test commands
#51605
Conversation
| /// | ||
| /// We support: | ||
| /// 1) Exact label matches (the '//' prefix will be stripped off). | ||
| /// 2) '/...' suffix which selects all targets that match the prefix. |
There was a problem hiding this comment.
Running Windows tests with this syntax is a bit verbose:
et test flutter/shell/platform/windows/...
Some potential solutions:
- We support executable names as selectors. Windows tests become
et test flutter_windows_unittests. - We support glob patterns. Windows tests become
et test **/windows/..., running all tests becomeset test **/... - We flatten the engine repo. Windows tests becomes
et test flutter/windows/...
Thoughts?
There was a problem hiding this comment.
What Bazel supports is being in a sub-directory, in other words:
cd shell/platform/windows
et test :flutter_windows_unittestsI'd be supportive of supporting that in et (assuming GN is OK with it or it's easy to implement). I don't think we should create any other cute/creative short-hands.
There was a problem hiding this comment.
I just press "up" (or ctrl+R) in my terminal when I'm developing so I don't totally get the point that it's a lot to type.
I agree we should support the sub-directory workflow that Matan mentioned.
Also, if we get into a situation where we have to specify a bunch of subdirectories (et build/test subA/... subB/...) the fix might be reorganize the directory tree so that related code is near each other.
| } | ||
| // Drop the empty line at the end of the output. | ||
| if (labels.isNotEmpty) { | ||
| assert(labels.last.isEmpty); |
There was a problem hiding this comment.
FYI: Neither the et wrapper script nor the unit tests run with --enable-asserts. Do you want to add the flag to the unit tests in this PR?
There was a problem hiding this comment.
I would also be supportive of just making this an unconditional throw, since it's going to throw anyway.
There was a problem hiding this comment.
If we never run with --enable-asserts can we add a lint that will flag assert calls for us, or, start running with asserts enabled?
Also, made this a throw.
|
This is looking awesome, excellent work! |
johnmccutchan
force-pushed
the
et_test
branch
4 times, most recently
from
ee91046 to
a1e3b83
Compare
- `et query tests` enumerates all test binaries encoded in BUILD.gn files. - `et test` (builds, then) runs a set of tests in parallel - Tests
johnmccutchan
added
the
autosubmit
…145946) flutter/engine@8b893cb...68aa9ba 2024-03-28 [email protected] [Impeller] dont clamp mipmap level to 0 with Vulkan textures. (flutter/engine#51761) 2024-03-28 [email protected] Add `et query tests` and `et test` commands (flutter/engine#51605) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
et query testsenumerates all test binaries encoded in BUILD.gn files.et testbuilds, then, runs a set of tests in parallel