Delete window_hooks_integration_test.dart #26756
Conversation
lib/ui/fixtures/ui_test.dart
Outdated
| @pragma('vm:entry-point') | ||
| void hooksTests() { | ||
| void test(String name, VoidCallback testFunction) { | ||
| print(name); |
There was a problem hiding this comment.
can we avoid printing unless the test fails? (ideally tests would have no output except to report failures or to say that all the tests passed)
| @@ -1,4 +1,3 @@ | |||
| // @dart = 2.6 | |||
There was a problem hiding this comment.
can you add a README to the fixtures directory that explains what it's for? (i'm assuming it's for development-time resources that don't end up in production binaries, but it's not clear exactly how we enforce that or why they are here rather than, say, in the testing directory)
There was a problem hiding this comment.
These directories exist in several places - the way that the C++ unit test setups work, the fixtures directory contains a Dart file and sometimes other data files like images for testing.
See also for example //shell/fixtures/shell_test.dart.
There's a GN template that defines how this works called test_fixtures that has a bunch of commentary on it: https://github.com/flutter/engine/blob/master/testing/testing.gni#L230-L250
There was a problem hiding this comment.
That commentary never actually says what it means by "fixtures". I'm also not sure how I'd ever find that if I started from a fixtures directory. Having some breadcrumbs would really help people orient themselves when first being exposed to this stuff, I think.
There was a problem hiding this comment.
Added a readme explaing a bit and pointing to the BUILD file and the template.
|
@Hixie I think your comments are addressed. Another review pass? |
|
My comments were just drive-by comments, I have no opinion on the overall change here. |
|
@dnfield The README is empty. Did you perhaps not stage the contents? |
chinmaygarde
left a comment
There was a problem hiding this comment.
Other than the README being empty, LGTM.
|
Hah, forgot to save the buffer before committing. Fixed that. Thanks for the review. |
|
This pull request is not suitable for automatic merging in its current state.
|
Fixes flutter/flutter#27628
Moves all of those tests to the ui_unittests program. Creates a stub test that allows dart code to call an arbitrary private hook and do whatever checks it needs. No more needing to fake dart:ui and mirror exactly oddities it has, like whether or not or how it's versioned. The final straw on this for me was when I was working on migrating other tests to null safety and found I could not remove dart version hints from dart:ui files because of this test.