-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add @isTest annotation to testGesture #18311
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
Conversation
Notifies IDEs this is a test method, and helps e.g. the flutter plugin recognize test methods and display them in the structure view in intellij.
devoncarew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/cc @scheglov
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/gestures.dart'; | ||
| import 'package:meta/meta.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm surprised this didn't cause a test failure... i thought we had a script that caught uses of meta/meta.dart. Maybe it's limited to only the flutter/lib/src directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From looking, it looks like that script is just checking in some combination of flutter/lib/ and flutter/lib/src/.
Is there an issue w/ importing package:meta? We want he isTest annotation, which isn't currently exported from foundation (and, the annotation isn't something users will generally need to use).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issue. We check it in the framework because we wanted to reduce the surface area that depended explicitly on other packages so that we could easily adjust that. I was just surprised that the tests weren't included in that.
|
Why just this one? It seems weird to only do it for this one method. |
|
It's used to identify a method as a test function (from the docs: |
|
Should we also mark |
|
Yes, if they invoke |
Update engine.version update tests for TextStyle changes in engine (flutter#17982) * update tests for TextStyle changes in engine * roll engine, support Foreground on TextStyle * Support for saving Dart compilation trace on device (WIP) Expose foreground in framework TextStyle use identical instead of == Add @istest annotation to testGesture (flutter#18311) Notifies IDEs this is a test method, and helps e.g. the flutter plugin recognize test methods and display them in the structure view in intellij. Update gallery assets version with optipng (flutter#18327) Passing any to named params require the name of the parameter itself. (flutter#18361) Update typedef syntax to use Function notation and turn on lint for old notation. (flutter#18362) Now that Dart 1 is turned off, reapplying my change to turn on the prefer_generic_function_type_aliases analysis option, and fix all the typedefs to Dart 2 preferred syntax. Also eliminated the unused analysis_options_repo.yaml file and turned on public_member_api_docs in analysys_options.yaml. No logic changes, just changing the typedef syntax for all typedefs, and updating analysis options. More flexible timeout logic in flutter_test (flutter#18256) This should reduce the number of flakes without actually increasing the timeout, so we'll still find out quickly if a test is hanging. The numbers here might need tweaking. Maybe the default two seconds is too short for CI bots. merge/apply/lerp prefer foreground doc updates and update for copyWith
Notifies IDEs this is a test method. This helps e.g. the flutter intellij plugin recognize test methods and display them in the structure view.
cc @devoncarew