-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add integration_test template to create template #70240
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
|
Addresses first item in #68818 |
| import 'dart:async'; | ||
| import 'dart:convert'; | ||
| import 'dart:io'; | ||
|
|
||
| import 'package:flutter_driver/flutter_driver.dart'; | ||
|
|
||
| Future<void> main() async { | ||
| final FlutterDriver driver = await FlutterDriver.connect(); | ||
| final String data = await driver.requestData(null); | ||
| await driver.close(); | ||
| final Map<String, dynamic> result = jsonDecode(data); | ||
| exit(result['result'] == 'true' ? 0 : 1); | ||
| } |
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.
This can be simplified to just
import 'package:integration_test/integration_test_driver.dart';
Future<void> main() => integrationDriver();
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.
Done
| // This file is provided as a convenience for running integration tests via the | ||
| // flutter drive command. | ||
| // | ||
| // flutter drive -t integration_test/app_test.dart --driver integration_test/driver.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.
In the docs, we tell people to put this file in app/test_driver/integration_test.dart. Should we move this to match?
Also, if we're going to spell out --driver in the flag we should spell out --target as well
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'd rather avoid having the extra directory here that might encourage people to create driver tests we'd rather they not.
TBH, I'd rather avoid having this file at all if we can come up with som ebetter way to have the tool (or a tool) run these :)
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.
That makes sense. If we don't have a tool we should definitely update the documentation to match, but I'd very much like to avoid the need for this file as well 🙂. I'll try to come up with a rough prototype next week to see if it's feasible to include an executable in package:integration_test (don't think any packages in flutter/flutter/packages do so today)
|
|
||
| testWidgets('Counter increments smoke test', (WidgetTester tester) async { | ||
| // Build our app and trigger a frame. | ||
| app.main(); |
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 a bit curious about the best practice here. Is this better than calling app.main, or instead importing the root widget and using tester.pumpWidget?
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.
app.main will be the same regardless of what the root widget is. Whereas if you refactor your app to add a new root widget, you'd have to update the integration tests.
|
It would be nice if there was a test (not in the devicelab) that generated a project with the integration test option and then confirmed that it runs correctly. Will it work on the flutter-tester device? |
|
Added a test in 405919f |
jonahwilliams
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.
LGTM!
…)" (flutter#71660)" This reverts commit 8e05e59.
Adds a package:integration_test test to the create template, along with a driver script for convenience when running with Flutter driver.
@jonahwilliams - this may help illustrate why/whether we want to update the drive command for integration_test tests when running them. An alternative is creating a seperate tool we publish to pub for more convenient local running - as suggested in #66264
@jonahwilliams @jiahaog @csells