Skip to content

Add initial framework for integration testing. #4960

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 19, 2022
Merged

Add initial framework for integration testing. #4960

merged 4 commits into from
Dec 19, 2022

Conversation

@kenzieschmoll
Copy link
Member

@kenzieschmoll kenzieschmoll commented Dec 16, 2022

This PR adds

  • integration_tests/test/infra/test_app_driver.dart : this code was copied from the existing file flutter_test_driver.dart, but paired down to use only what we need right now. This code will be used to spin up a test app that we will connect to DevTools for integration testing.
  • test_driver/integration_test.dart : this is the test driver we will use for integration tests

@kenzieschmoll kenzieschmoll requested a review from a team as a code owner December 16, 2022 21:07
@kenzieschmoll kenzieschmoll requested review from CoderDake and polina-c and removed request for a team December 16, 2022 21:07
@polina-c
Copy link
Contributor

polina-c commented Dec 19, 2022

Why test_driver/integration_test.dart is not in test_infra?
Are you sure we want to have separate test_infra in integration_test? Would it be more convenient to have it shared between different types of testing?

Couple options:

  1. take test_infra out of test and integration_test and make it shared
  2. Declare that test is lower level than integration_test and use test/test-infra for both

bkonyi
bkonyi reviewed Dec 19, 2022
View reviewed changes
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DBR

packages/devtools_app/integration_test/test_infra/test_app_driver.dart Outdated
Completer<Map<String, dynamic>>();
late StreamSubscription<String> sub;
sub = stdoutController.stream.listen((String line) async {
final dynamic json = _parseFlutterResponse(line);
Copy link
Contributor

@bkonyi bkonyi Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this type necessary? _parseFlutterResponse has a known, non-dynamic return type.

Copy link
Member Author

@kenzieschmoll kenzieschmoll Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not. All this code was written in pre-null safe Dart land and copied over in this PR. updated.

packages/devtools_app/integration_test/test_infra/test_app_driver.dart Outdated
(id != null && json['id'] == id)) {
await sub.cancel();
response.complete(json);
} else if (!ignoreAppStopEvent && json['event'] == 'app.stop') {
Copy link
Contributor

@bkonyi bkonyi Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have constants defined in devtools_shared for these daemon event names?

Copy link
Member Author

@kenzieschmoll kenzieschmoll Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created enum FlutterDaemonConstants for now with a TODO to consider placing these constants in devtools_shared or in flutter depending on where they would be used.

@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Dec 19, 2022

Why test_driver/integration_test.dart is not in test_infra? Are you sure we want to have separate test_infra in integration_test? Would it be more convenient to have it shared between different types of testing?

Couple options:

  1. take test_infra out of test and integration_test and make it shared
  2. Declare that test is lower level than integration_test and use test/test-infra for both

Official Flutter guidance recommends code structure:

lib/
  ...
integration_test/
test/

I did try to create a shared test folder, but libraries under test/ and integration_test/ could not import anything outside of their direct folders or lib/.

Unit tests and integration tests will have some different "test_infra" so it is okay to have two folders. Once we build out our integration tests more fully, it may make sense to put anything that could be shared between test/ and integration_test into packages/devtools_test so that both test/ and integration_test can share it. This can be done at a later time though when we have a better idea of what will need to be shared.

@kenzieschmoll kenzieschmoll merged commit 8d0db80 into flutter:master Dec 19, 2022
@kenzieschmoll kenzieschmoll deleted the integration-test-1 branch December 19, 2022 22:08
@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Dec 19, 2022

Work towards #4934

@CoderDake CoderDake mentioned this pull request Dec 22, 2022
Closed
64 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkonyi bkonyi bkonyi left review comments

@polina-c polina-c polina-c approved these changes

@CoderDake CoderDake Awaiting requested review from CoderDake CoderDake is a code owner automatically assigned from flutter/devtools-reviewers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kenzieschmoll @polina-c @bkonyi