Allow specifying both Dart and non-Dart fixtures in engine unit-tests. #9113
Conversation
This fixes numerous issues in the way in which fixtures were managed in the engine unit-tests. * Instead of only being able to specify Dart fixtures, unit-tests may specify non-Dart fixtures as well. These are simply copied over to the fixtures directory known to the unit-test at runtime. * An issue where numerous Dart files could be given to the kernel snapshotter has been addressed. It was anticipated that such a (legal) invocation to the kernel snapshotter would produce a snapshot with the contents of all the Dart files added to the root library. This is incorrect and the behavior in this case is undefined. * Dart files referenced by the main Dart file are correctly tracked via a depfile. * The snapshotter arguments have been cleaned up to get rid of unused arguments (`—strong`) and the use of the VM product mode argument has been corrected to no longer depend on the Flutter product mode.
chinmaygarde
requested review from
GaryQian,
cbracken,
jason-simmons and
liyuqian
|
cc @rmacnak-google who helped audit the snapshotter arguments. |
testing/testing.gni
Outdated
| # | ||
| # Arguments: | ||
| # | ||
| # fixtures (required): The list of test fixtures. An empty list may be |
There was a problem hiding this comment.
I wonder why we must provide an empty list []? For example, is
test_fixtures("runtime_fixtures") {
dart_main = "fixtures/runtime_test.dart"
fixtures = []
}
a little unnecessarily verbose and confusing than
test_fixtures("runtime_fixtures") {
dart_main = "fixtures/runtime_test.dart"
}
?
If there's a nice reason to put an empty list here, maybe it's worth to write down that reason in this documentation.
There was a problem hiding this comment.
The reason for the empty list is a GN quirk where if the template expansion does not need any arguments (as it is in this case where the target_gen_dir used to compute the fixtures directory), the invoker scope goes unused in the expansion and GN warns of this and errors out.
There was a problem hiding this comment.
You can have either one or both the arguments in the template and it will work fine. If you have none, then you'll see the unused invoker scope error.
There was a problem hiding this comment.
I see, thanks! It would be nice to add You can have either one or both the arguments in the template and it will work fine. If you have none, then you'll see the unused invoker scope error. to the comment so in a few months for now, I can still figure this out by reading the comments instead of git blame 😄
testing/testing.gni
Outdated
| # | ||
| # Arguments: | ||
| # | ||
| # fixtures (required): The list of test fixtures. An empty list may be |
There was a problem hiding this comment.
I see, thanks! It would be nice to add You can have either one or both the arguments in the template and it will work fine. If you have none, then you'll see the unused invoker scope error. to the comment so in a few months for now, I can still figure this out by reading the comments instead of git blame 😄
6 participants
This fixes numerous issues in the way in which fixtures were managed
in the engine unit-tests.
non-Dart fixtures as well. These are simply copied over to the fixtures
directory known to the unit-test at runtime.
has been addressed. It was anticipated that such a (legal) invocation to the
kernel snapshotter would produce a snapshot with the contents of all the Dart
files added to the root library. This is incorrect and the behavior in this
case is undefined.
depfile.
arguments (
—strong) and the use of the VM product mode argument has beencorrected to no longer depend on the Flutter product mode.