Skip to content

Migrate asset_handler_test to null-safety #1710

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

Merged
merged 4 commits into from
Aug 4, 2022
Merged

Migrate asset_handler_test to null-safety #1710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca81036
Simplify test
elliette Aug 2, 2022
939b8f9
fix types
elliette Aug 3, 2022
ec3cea7
Clean up
elliette Aug 4, 2022
d388dee
Throw state error
elliette Aug 4, 2022
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
6 changes: 3 additions & 3 deletions dwds/lib/src/dwds_vm_client.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ final _logger = Logger('DwdsVmClient');
class DwdsVmClient {
final VmService client;
final StreamController<Map<String, Object>> _requestController;
final StreamController<Map<String, Object>> _responseController;
final StreamController<Map<String, Object?>> _responseController;

static const int kFeatureDisabled = 100;
static const String kFeatureDisabledMessage = 'Feature is disabled.';
Expand All @@ -44,7 +44,7 @@ class DwdsVmClient {
DebugService debugService, DwdsStats dwdsStats) async {
// Set up hot restart as an extension.
final requestController = StreamController<Map<String, Object>>();
final responseController = StreamController<Map<String, Object>>();
final responseController = StreamController<Map<String, Object?>>();
VmServerConnection(requestController.stream, responseController.sink,
debugService.serviceExtensionRegistry, debugService.chromeProxyService);
final client =
Expand All @@ -55,7 +55,7 @@ class DwdsVmClient {
'$request');
return;
}
requestController.sink.add(jsonDecode(request) as Map<String, Object>);
requestController.sink.add(Map<String, Object>.from(jsonDecode(request)));
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it is necessary to create a new object here (possible performance hit?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure of another way - jsonDecode returns dynamic, so using as to cast is throwing a type error (as should only be used "if and only if you are sure that the object is of that type": https://dart.dev/guides/language/language-tour#type-test-operators)

});
final chromeProxyService =
debugService.chromeProxyService as ChromeProxyService;
Expand Down
22 changes: 11 additions & 11 deletions dwds/lib/src/services/debug_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ int _clientsConnected = 0;

Logger _logger = Logger('DebugService');

void Function(WebSocketChannel, String) _createNewConnectionHandler(
void Function(WebSocketChannel) _createNewConnectionHandler(
ChromeProxyService chromeProxyService,
ServiceExtensionRegistry serviceExtensionRegistry, {
void Function(Map<String, dynamic>)? onRequest,
void Function(Map<String, dynamic>)? onResponse,
void Function(Map<String, Object>)? onRequest,
void Function(Map<String, Object?>)? onResponse,
}) {
return (webSocket, protocol) {
final responseController = StreamController<Map<String, Object>>();
return (webSocket) {
final responseController = StreamController<Map<String, Object?>>();
webSocket.sink.addStream(responseController.stream.map((response) {
if (onResponse != null) onResponse(response);
return jsonEncode(response);
Expand All @@ -53,7 +53,7 @@ void Function(WebSocketChannel, String) _createNewConnectionHandler(
'Got value with unexpected type ${value.runtimeType} from web '
'socket, expected a List<int> or String.');
}
final request = jsonDecode(value as String) as Map<String, Object>;
final request = Map<String, Object>.from(jsonDecode(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the new object necessary here as well?

if (onRequest != null) onRequest(request);
return request;
});
Expand All @@ -76,12 +76,12 @@ Future<void> _handleSseConnections(
SseHandler handler,
ChromeProxyService chromeProxyService,
ServiceExtensionRegistry serviceExtensionRegistry, {
void Function(Map<String, dynamic>)? onRequest,
void Function(Map<String, dynamic>)? onResponse,
void Function(Map<String, Object>)? onRequest,
void Function(Map<String, Object?>)? onResponse,
}) async {
while (await handler.connections.hasNext) {
final connection = await handler.connections.next;
final responseController = StreamController<Map<String, Object>>();
final responseController = StreamController<Map<String, Object?>>();
final sub = responseController.stream.map((response) {
if (onResponse != null) onResponse(response);
return jsonEncode(response);
Expand Down Expand Up @@ -214,8 +214,8 @@ class DebugService {
LoadStrategy loadStrategy,
AppConnection appConnection,
UrlEncoder? urlEncoder, {
void Function(Map<String, dynamic>)? onRequest,
void Function(Map<String, dynamic>)? onResponse,
void Function(Map<String, Object>)? onRequest,
void Function(Map<String, Object?>)? onResponse,
bool spawnDds = true,
bool useSse = false,
ExpressionCompiler? expressionCompiler,
Expand Down
21 changes: 13 additions & 8 deletions dwds/test/fixtures/context.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class TestContext {
WebkitDebugger get webkitDebugger => _webkitDebugger!;
late WebkitDebugger? _webkitDebugger;

Handler get assetHandler => _assetHandler!;
late Handler? _assetHandler;

Client get client => _client!;
late Client? _client;

Expand Down Expand Up @@ -203,7 +206,6 @@ class TestContext {

ExpressionCompiler? expressionCompiler;
AssetReader assetReader;
Handler assetHandler;
Stream<BuildResults> buildResults;
RequireStrategy requireStrategy;
String basePath = '';
Expand Down Expand Up @@ -237,7 +239,7 @@ class TestContext {
.timeout(const Duration(seconds: 60));

final assetServerPort = daemonPort(workingDirectory);
assetHandler = proxyHandler(
_assetHandler = proxyHandler(
'http://localhost:$assetServerPort/$pathToServe/',
client: client);
assetReader =
Expand Down Expand Up @@ -291,7 +293,7 @@ class TestContext {

basePath = webRunner.devFS.assetServer.basePath;
assetReader = webRunner.devFS.assetServer;
assetHandler = webRunner.devFS.assetServer.handleRequest;
_assetHandler = webRunner.devFS.assetServer.handleRequest;

requireStrategy = FrontendServerRequireStrategyProvider(
reloadConfiguration, assetReader, () async => {}, basePath)
Expand Down Expand Up @@ -353,11 +355,14 @@ class TestContext {
: 'http://localhost:$port/$basePath/$path';

await _webDriver?.get(appUrl);
final tab = await (connection.getTab((t) => t.url == appUrl)
as FutureOr<ChromeTab>);
_tabConnection = await tab.connect();
await tabConnection.runtime.enable();
await tabConnection.debugger.enable();
final tab = await connection.getTab((t) => t.url == appUrl);
if (tab != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should throw if tab is not found, the tests won't run properly in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done!

_tabConnection = await tab.connect();
await tabConnection.runtime.enable();
await tabConnection.debugger.enable();
} else {
throw StateError('Unable to connect to tab.');
}

if (enableDebugExtension) {
final extensionTab = await _fetchDartDebugExtensionTab(connection);
Expand Down
27 changes: 2 additions & 25 deletions dwds/test/handlers/asset_handler_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,41 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

// @dart = 2.9

import 'dart:io';

import 'package:http/http.dart' as http;
import 'package:http/io_client.dart';
import 'package:shelf/shelf.dart';
import 'package:shelf_proxy/shelf_proxy.dart';
import 'package:test/test.dart';

import '../fixtures/context.dart';
import '../fixtures/logging.dart';
import '../fixtures/utilities.dart';

void main() {
group('Asset handler', () {
final context = TestContext();
Handler assetHandler;
http.Client client;

setUpAll(() async {
setCurrentLogWriter();
await context.setUp(
enableExpressionEvaluation: true,
verboseCompiler: false,
);

client = IOClient(HttpClient()
..maxConnectionsPerHost = 200
..idleTimeout = const Duration(seconds: 30)
..connectionTimeout = const Duration(seconds: 30));

final assetServerPort = daemonPort(context.workingDirectory);
final pathToServe = context.pathToServe;

assetHandler = proxyHandler(
'http://localhost:$assetServerPort/$pathToServe/',
client: client);
});

tearDownAll(() async {
client.close();
await context.tearDown();
});

setUp(setCurrentLogWriter);

Future<void> readAsString(String path) async {
final request = Request('GET', Uri.parse('http://foo:0000/$path'));
final response = await assetHandler(request);
final response = await context.assetHandler(request);
final result = await response.readAsString();
expect(result, isNotNull,
reason: 'Failed to read $path: ${response.statusCode}');
}

Future<void> readAsBytes(String path) async {
final request = Request('GET', Uri.parse('http://foo:0000/$path'));
final response = await assetHandler(request);
final response = await context.assetHandler(request);
final result = await response.read().toList();
expect(result, isNotNull,
reason: 'Failed to read $path: ${response.statusCode}');
Expand Down