Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Fix webview_flutter Android integration tests and add Espresso #4147

Merged
merged 25 commits into from
Jul 26, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
18e6d22
lets see if integration tests run
bparrishMines Jul 8, 2021
2025b84
change folder name
bparrishMines Jul 12, 2021
ddf5d91
create a running test with ./gradlew app:connectedAndroidTest -Ptarge…
bparrishMines Jul 13, 2021
562cc4d
see if tests run on ci?.
bparrishMines Jul 14, 2021
cad641b
fix a few tests
bparrishMines Jul 17, 2021
b94cfbb
try get all tests passing
bparrishMines Jul 17, 2021
744c395
fluttertestrunner and placeholder test
bparrishMines Jul 18, 2021
6d9f282
formatting
bparrishMines Jul 18, 2021
e5ea80c
fix blank test, formatting, license
bparrishMines Jul 18, 2021
2489c44
undo formatting change and remove navigation delegate
bparrishMines Jul 18, 2021
76d612e
Improve example test and version bump test dependencies
bparrishMines Jul 19, 2021
dc6be9a
remove assertequals
bparrishMines Jul 19, 2021
8713690
small format changes
bparrishMines Jul 19, 2021
648e94c
license
bparrishMines Jul 19, 2021
3619762
remove truth dependency, change core dep and follow example
bparrishMines Jul 19, 2021
ffe0058
undo the test change
bparrishMines Jul 19, 2021
f624087
Merge branch 'master' of github.com:flutter/plugins into androidTest
bparrishMines Jul 20, 2021
a10bfde
readd analysisoptions
bparrishMines Jul 20, 2021
42b7238
remove top line
bparrishMines Jul 20, 2021
927b9fe
empty commit to check for flakes
bparrishMines Jul 20, 2021
089fa76
skip tests
bparrishMines Jul 22, 2021
77b45f1
Merge branch 'master' of github.com:flutter/plugins into androidTest
bparrishMines Jul 22, 2021
7e73bb1
remove webview_flutter
bparrishMines Jul 22, 2021
79d82c4
Merge branch 'master' of github.com:flutter/plugins into androidTest
bparrishMines Jul 22, 2021
d34c705
Merge branch 'master' into androidTest
bparrishMines Jul 24, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions packages/webview_flutter/example/android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ flutter {

dependencies {
testImplementation 'junit:junit:4.12'
androidTestImplementation 'androidx.test:runner:1.2.0'
androidTestImplementation 'androidx.test:rules:1.2.0'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
testImplementation "com.google.truth:truth:1.0"
Copy link

Choose a reason for hiding this comment

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

this doesn't seem used, is it?

androidTestImplementation 'androidx.test:runner:1.1.1'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.1.1'
api 'androidx.test:core:1.2.0'
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@
@RunWith(FlutterTestRunner.class)
public class MainActivityTest {
@Rule
public ActivityTestRule<FlutterActivity> rule = new ActivityTestRule<>(FlutterActivity.class);
public final ActivityTestRule<FlutterActivity> rule =
new ActivityTestRule<>(FlutterActivity.class);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.plugins.webviewflutterexample;

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class WebViewTest {
Copy link

Choose a reason for hiding this comment

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

what runner is this using? do you want to use AndroidJUnit4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

sgtm

@Test
public void placeHolderTest() {
assertEquals(1, 1);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<application
android:icon="@mipmap/ic_launcher"
android:label="webview_flutter_example"
android:usesCleartextTraffic="true"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the espresso package

android:name="io.flutter.app.FlutterApplication">
<meta-data
android:name="flutterEmbedding"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import 'package:integration_test/integration_test.dart';
void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();

testWidgets('initalUrl', (WidgetTester tester) async {
testWidgets('initialUrl', (WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Completer<WebViewController>();
await tester.pumpWidget(
Expand Down Expand Up @@ -532,6 +532,7 @@ void main() {
expect(fullScreen, _webviewBool(false));
});

// allowsInlineMediaPlayback is a noop on Android, so it is skipped.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/flutter/plugins/blob/master/packages/webview_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebView.java#L397

This test fails when running on Android, but it doesn't seem like there is anything we can do since a flag like this isn't supported on Android. At least according to the comment.

testWidgets(
'Video plays full screen when allowsInlineMediaPlayback is false',
(WidgetTester tester) async {
Expand Down Expand Up @@ -581,7 +582,7 @@ void main() {
String fullScreen =
await controller.evaluateJavascript('isFullScreen();');
expect(fullScreen, _webviewBool(true));
});
}, skip: Platform.isAndroid);
});

group('Audio playback policy', () {
Expand Down Expand Up @@ -1272,18 +1273,19 @@ void main() {
),
);
final WebViewController controller = await controllerCompleter.future;
await controller.evaluateJavascript('window.open("about:blank", "_blank")');
await controller
.evaluateJavascript('window.open("https://flutter.dev/", "_blank")');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test would stall on Android. For some reason, Android has different behavior when a window.open is called with a url with https:// vs not. It loads normally when using a URL with https://, but appends the url to the current url when it doesnt. (e.g. https://flutter.dev/ turns into https://flutter.dev/about:blank)

await pageLoaded.future;
final String? currentUrl = await controller.currentUrl();
expect(currentUrl, 'about:blank');
expect(currentUrl, 'https://flutter.dev/');
});

testWidgets(
'can open new window and go back',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the weird bug where pageLoaded.complete() was called in the callback multiple times. From the Dart side, this caused a silent error and prevented the rest of the method calls from going through.

(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Completer<WebViewController>();
final Completer<void> pageLoaded = Completer<void>();
Completer<void> pageLoaded = Completer<void>();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
Expand All @@ -1301,13 +1303,20 @@ void main() {
),
);
final WebViewController controller = await controllerCompleter.future;
expect(controller.currentUrl(), completion('https://flutter.dev/'));
await pageLoaded.future;
pageLoaded = Completer<void>();
Copy link

Choose a reason for hiding this comment

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

can it have a different name, so there's no need to reassign a new completer?


await controller
.evaluateJavascript('window.open("https://www.google.com")');
.evaluateJavascript('window.open("https://www.google.com/")');
await pageLoaded.future;
pageLoaded = Completer<void>();
Copy link

Choose a reason for hiding this comment

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

ditto

expect(controller.currentUrl(), completion('https://www.google.com/'));

expect(controller.canGoBack(), completion(true));
await controller.goBack();
expect(controller.currentUrl(), completion('https://www.flutter.dev'));
await pageLoaded.future;
expect(controller.currentUrl(), completion('https://flutter.dev/'));
},
skip: !Platform.isAndroid,
);
Expand Down
1 change: 1 addition & 0 deletions packages/webview_flutter/example/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ dependencies:
path: ../

dev_dependencies:
espresso: ^0.1.0+2
Copy link

Choose a reason for hiding this comment

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

I don't think this is used. is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not in this PR, but I added it as part of "adding the test harness". I assumed that when another user wanted to create a PR and needed to add a test, that we didn't want to require them to add dependencies.

Copy link

Choose a reason for hiding this comment

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

Sounds good

flutter_test:
sdk: flutter
flutter_driver:
Expand Down