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

[url_launcher] Migrate unit tests to NNBD #3657

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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

This file was deleted.

2 changes: 1 addition & 1 deletion packages/url_launcher/url_launcher/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ dev_dependencies:
flutter_test:
sdk: flutter
test: ^1.16.3
mockito: ^5.0.0-nullsafety.7
mockito: ^5.0.0
plugin_platform_interface: ^2.0.0
pedantic: ^1.10.0

Expand Down
108 changes: 53 additions & 55 deletions packages/url_launcher/url_launcher/test/link_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// TODO(egarciad): Remove once Mockito has been migrated to null safety.
// @dart = 2.9

import 'dart:ui';

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
import 'package:flutter/services.dart';
import 'package:plugin_platform_interface/plugin_platform_interface.dart';
import 'package:url_launcher/link.dart';
import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart';

import 'mock_url_launcher_platform.dart';

final MethodCodec _codec = const JSONMethodCodec();

void main() {
final MockUrlLauncher mock = MockUrlLauncher();
UrlLauncherPlatform.instance = mock;
late MockUrlLauncher mock;

PlatformMessageCallback realOnPlatformMessage;
PlatformMessageCallback? realOnPlatformMessage;
setUp(() {
mock = MockUrlLauncher();
UrlLauncherPlatform.instance = mock;
realOnPlatformMessage = window.onPlatformMessage;
});
tearDown(() {
Expand All @@ -31,11 +31,11 @@ void main() {
group('$Link', () {
testWidgets('handles null uri correctly', (WidgetTester tester) async {
bool isBuilt = false;
FollowLink followLink;
FollowLink? followLink;

final Link link = Link(
uri: null,
builder: (BuildContext context, FollowLink followLink2) {
builder: (BuildContext context, FollowLink? followLink2) {
isBuilt = true;
followLink = followLink2;
return Container();
Expand All @@ -50,66 +50,62 @@ void main() {

testWidgets('calls url_launcher for external URLs with blank target',
(WidgetTester tester) async {
FollowLink followLink;
FollowLink? followLink;

await tester.pumpWidget(Link(
uri: Uri.parse('http://example.com/foobar'),
target: LinkTarget.blank,
builder: (BuildContext context, FollowLink followLink2) {
builder: (BuildContext context, FollowLink? followLink2) {
followLink = followLink2;
return Container();
},
));

when(mock.canLaunch('http://example.com/foobar'))
.thenAnswer((realInvocation) => Future<bool>.value(true));
clearInteractions(mock);
await followLink();

verifyInOrder([
mock.canLaunch('http://example.com/foobar'),
mock.launch(
'http://example.com/foobar',
mock
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to migrate away from "when...` in mockito and introduce our own mocks? mockito already migrated to stable we should be able to just use it with nnbd, right?

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

Choose a reason for hiding this comment

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

Do we prefer auto-gen or manual? The doc mentioned autogen is recommended and I see that you manually generated the mocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Mockito generation adds a bunch more moving pieces; it didn't seem worth adding that extra complexity for a simple class with a couple of methods. There may be cases where we want to do it, but if it's simple to just start from Fake and build manually that seems easier.

This one is probably borderline since it's doing real mock validation rather than just faking; in a number of other cases we were using Mockito to make fakes, not actual mocks. We may want to give it a try for the remaining unit tests that need conversion and compare (or I can try it with this one if you'd like).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll try with android_intent

..setLaunchExpectations(
url: 'http://example.com/foobar',
useSafariVC: false,
useWebView: false,
universalLinksOnly: false,
enableJavaScript: false,
enableDomStorage: false,
headers: <String, String>{},
webOnlyWindowName: null,
)
]);
..setResponse(true);
await followLink!();
expect(mock.canLaunchCalled, isTrue);
expect(mock.launchCalled, isTrue);
});

testWidgets('calls url_launcher for external URLs with self target',
(WidgetTester tester) async {
FollowLink followLink;
FollowLink? followLink;

await tester.pumpWidget(Link(
uri: Uri.parse('http://example.com/foobar'),
target: LinkTarget.self,
builder: (BuildContext context, FollowLink followLink2) {
builder: (BuildContext context, FollowLink? followLink2) {
followLink = followLink2;
return Container();
},
));

when(mock.canLaunch('http://example.com/foobar'))
.thenAnswer((realInvocation) => Future<bool>.value(true));
clearInteractions(mock);
await followLink();

verifyInOrder([
mock.canLaunch('http://example.com/foobar'),
mock.launch(
'http://example.com/foobar',
mock
..setLaunchExpectations(
url: 'http://example.com/foobar',
useSafariVC: true,
useWebView: true,
universalLinksOnly: false,
enableJavaScript: false,
enableDomStorage: false,
headers: <String, String>{},
webOnlyWindowName: null,
)
]);
..setResponse(true);
await followLink!();
expect(mock.canLaunchCalled, isTrue);
expect(mock.launchCalled, isTrue);
});

testWidgets('sends navigation platform messages for internal route names',
Expand All @@ -125,21 +121,21 @@ void main() {
final List<MethodCall> frameworkCalls = <MethodCall>[];
window.onPlatformMessage = (
String name,
ByteData data,
PlatformMessageResponseCallback callback,
ByteData? data,
PlatformMessageResponseCallback? callback,
) {
frameworkCalls.add(_codec.decodeMethodCall(data));
realOnPlatformMessage(name, data, callback);
realOnPlatformMessage!(name, data, callback);
};

final Uri uri = Uri.parse('/foo/bar');
FollowLink followLink;
FollowLink? followLink;

await tester.pumpWidget(MaterialApp(
routes: <String, WidgetBuilder>{
'/': (BuildContext context) => Link(
uri: uri,
builder: (BuildContext context, FollowLink followLink2) {
builder: (BuildContext context, FollowLink? followLink2) {
followLink = followLink2;
return Container();
},
Expand All @@ -150,11 +146,11 @@ void main() {

engineCalls.clear();
frameworkCalls.clear();
clearInteractions(mock);
await followLink();
await followLink!();

// Shouldn't use url_launcher when uri is an internal route name.
verifyZeroInteractions(mock);
expect(mock.canLaunchCalled, isFalse);
expect(mock.launchCalled, isFalse);

// A message should've been sent to the engine (by the Navigator, not by
// the Link widget).
Expand Down Expand Up @@ -191,19 +187,19 @@ void main() {
final List<MethodCall> frameworkCalls = <MethodCall>[];
window.onPlatformMessage = (
String name,
ByteData data,
PlatformMessageResponseCallback callback,
ByteData? data,
PlatformMessageResponseCallback? callback,
) {
frameworkCalls.add(_codec.decodeMethodCall(data));
realOnPlatformMessage(name, data, callback);
realOnPlatformMessage!(name, data, callback);
};

final Uri uri = Uri.parse('/foo/bar');
FollowLink followLink;
FollowLink? followLink;

final Link link = Link(
uri: uri,
builder: (BuildContext context, FollowLink followLink2) {
builder: (BuildContext context, FollowLink? followLink2) {
followLink = followLink2;
return Container();
},
Expand All @@ -217,11 +213,11 @@ void main() {

engineCalls.clear();
frameworkCalls.clear();
clearInteractions(mock);
await followLink();
await followLink!();

// Shouldn't use url_launcher when uri is an internal route name.
verifyZeroInteractions(mock);
expect(mock.canLaunchCalled, isFalse);
expect(mock.launchCalled, isFalse);

// Sends route information update to the engine.
expect(engineCalls, hasLength(1));
Expand Down Expand Up @@ -249,10 +245,6 @@ void main() {
});
}

class MockUrlLauncher extends Mock
with MockPlatformInterfaceMixin
implements UrlLauncherPlatform {}

class MockRouteInformationParser extends Mock
implements RouteInformationParser<bool> {
@override
Expand All @@ -261,13 +253,19 @@ class MockRouteInformationParser extends Mock
}
}

class MockRouterDelegate extends Mock implements RouterDelegate {
MockRouterDelegate({@required this.builder});
class MockRouterDelegate extends Mock implements RouterDelegate<Object> {
MockRouterDelegate({required this.builder});

final WidgetBuilder builder;

@override
Widget build(BuildContext context) {
return builder(context);
}

@override
Future<void> setInitialRoutePath(Object configuration) async {}

@override
Future<void> setNewRoutePath(Object configuration) async {}
}
Loading