Skip to content

Migrate debugger_test and instance_test to null-safety #1708

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 9 commits into from
Aug 8, 2022
Merged

Migrate debugger_test and instance_test to null-safety #1708

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
28eb69e
Migrate tests to null safety
elliette Aug 1, 2022
28a10a0
Format
elliette Aug 1, 2022
0f4dd9c
Fix analyze errors
elliette Aug 1, 2022
6166627
Merge in master
elliette Aug 5, 2022
aa0d48f
debugger test passes
elliette Aug 5, 2022
6179ee7
Revert inspector test
elliette Aug 6, 2022
2e57e8b
Fix instance_test
elliette Aug 8, 2022
63d7934
Add new line back in
elliette Aug 8, 2022
91c2634
Test is working
elliette Aug 8, 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
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/debugger.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class Debugger extends Domain {
});

handleErrorIfPresent(await _remoteDebugger.enablePage());
handleErrorIfPresent(await _remoteDebugger.enable() as WipResponse);
await _remoteDebugger.enable();

// Enable collecting information about async frames when paused.
handleErrorIfPresent(await _remoteDebugger
Expand Down
45 changes: 9 additions & 36 deletions dwds/test/debugger_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// 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

@TestOn('vm')
import 'dart:async';

Expand All @@ -23,12 +21,12 @@ import 'fixtures/debugger_data.dart';
import 'fixtures/fakes.dart';

final context = TestContext();
AppInspector inspector;
Debugger debugger;
FakeWebkitDebugger webkitDebugger;
StreamController<DebuggerPausedEvent> pausedController;
Locations locations;
SkipLists skipLists;
late AppInspector inspector;
late Debugger debugger;
late FakeWebkitDebugger webkitDebugger;
late StreamController<DebuggerPausedEvent> pausedController;
late Locations locations;
late SkipLists skipLists;

class TestStrategy extends FakeStrategy {
@override
Expand Down Expand Up @@ -92,7 +90,7 @@ void main() async {
skipLists = SkipLists();
debugger = await Debugger.create(
webkitDebugger,
null,
(_, __) {},
locations,
skipLists,
root,
Expand All @@ -109,8 +107,8 @@ void main() async {
expect(frames, isNotNull);
expect(frames, isNotEmpty);

final firstFrame = frames[0];
final frame1Variables = firstFrame.vars.map((each) => each.name).toList();
final firstFrameVars = frames[0].vars!;
final frame1Variables = firstFrameVars.map((each) => each.name).toList();
expect(frame1Variables, ['a', 'b']);
});

Expand Down Expand Up @@ -163,29 +161,4 @@ void main() async {
expect(frames[3].kind, FrameKind.kAsyncSuspensionMarker);
expect(frames[4].kind, FrameKind.kAsyncCausal);
});

group('errors', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed because all this was checking was whether an error would be thrown if a null value was passed as a parameter (when a null value wasn't expected). With null-safety, this isn't possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea was to test that the error message is shown, passing the null was just the way to trigger it. Is there any other way to trigger the errors in the debugger? Maybe throwing somewhere in the fake inspector code? Or by introducing a fake stack computer that throws when calculating frames?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test back in, thanks!

setUp(() {
// We need to provide an Isolate so that the code doesn't bail out on a null
// check before it has a chance to throw.
inspector = FakeInspector(fakeIsolate: simpleIsolate);
debugger.updateInspector(inspector);
});

test('errors in the zone are caught and logged', () async {
// Add a DebuggerPausedEvent with a null parameter to provoke an error.
pausedController.sink.add(DebuggerPausedEvent({
'params': {
'reason': 'other',
'callFrames': [
null,
],
}
}));
expect(
Debugger.logger.onRecord,
emitsThrough(predicate(
(log) => log.message == 'Error calculating Dart frames')));
});
});
}
25 changes: 15 additions & 10 deletions dwds/test/fixtures/fakes.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ class FakeWebkitDebugger implements WebkitDebugger {
Stream<TargetCrashedEvent> get onTargetCrashed => Stream.empty();

@override
Future<WipResponse> pause() async => WipResponse({});
Future<WipResponse> pause() async => fakeWipResponse;

@override
Future<WipResponse> resume() async => WipResponse({});
Future<WipResponse> resume() async => fakeWipResponse;

@override
Map<String, WipScript> get scripts => _scripts!;
Expand All @@ -200,27 +200,27 @@ class FakeWebkitDebugger implements WebkitDebugger {
if (command == 'Runtime.getProperties') {
return results[resultsReturned++];
}
return WipResponse({});
return fakeWipResponse;
}

@override
Future<WipResponse> setPauseOnExceptions(PauseState state) async =>
WipResponse({});
fakeWipResponse;

@override
Future<WipResponse> removeBreakpoint(String breakpointId) async =>
WipResponse({});
fakeWipResponse;

@override
Future<WipResponse> stepInto({Map<String, dynamic>? params}) async =>
WipResponse({});
fakeWipResponse;

@override
Future<WipResponse> stepOut() async => WipResponse({});
Future<WipResponse> stepOut() async => fakeWipResponse;

@override
Future<WipResponse> stepOver({Map<String, dynamic>? params}) async =>
WipResponse({});
fakeWipResponse;

@override
Stream<ConsoleAPIEvent> get onConsoleAPICalled => Stream.empty();
Expand Down Expand Up @@ -251,10 +251,10 @@ class FakeWebkitDebugger implements WebkitDebugger {
[];

@override
Future<WipResponse> enablePage() async => WipResponse({});
Future<WipResponse> enablePage() async => fakeWipResponse;

@override
Future<WipResponse> pageReload() async => WipResponse({});
Future<WipResponse> pageReload() async => fakeWipResponse;
}

/// Fake execution context that is needed for id only
Expand Down Expand Up @@ -365,3 +365,8 @@ class FakeAssetReader implements AssetReader {
return contents;
}
}

final fakeWipResponse = WipResponse({
'id': 1,
'result': {'fake': ''}
});
86 changes: 43 additions & 43 deletions dwds/test/instance_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// 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:async';

import 'package:dwds/src/connections/debug_connection.dart';
import 'package:dwds/src/debugging/debugger.dart';
Expand All @@ -20,8 +20,8 @@ final context = TestContext(
WipConnection get tabConnection => context.tabConnection;

void main() {
AppInspector inspector;
Debugger debugger;
late AppInspector inspector;
late Debugger debugger;

setUpAll(() async {
await context.setUp();
Expand Down Expand Up @@ -54,9 +54,9 @@ void main() {
final remoteObject = await libraryPublicFinal();
final nullVariable = await inspector.loadField(remoteObject, 'notFinal');
final ref = await inspector.instanceRefFor(nullVariable);
expect(ref.valueAsString, 'null');
expect(ref!.valueAsString, 'null');
expect(ref.kind, InstanceKind.kNull);
final classRef = ref.classRef;
final classRef = ref.classRef!;
expect(classRef.name, 'Null');
expect(classRef.id, 'classes|dart:core|Null');
});
Expand All @@ -65,9 +65,9 @@ void main() {
final remoteObject = await libraryPublicFinal();
final count = await inspector.loadField(remoteObject, 'count');
final ref = await inspector.instanceRefFor(count);
expect(ref.valueAsString, '0');
expect(ref!.valueAsString, '0');
expect(ref.kind, InstanceKind.kDouble);
final classRef = ref.classRef;
final classRef = ref.classRef!;
expect(classRef.name, 'Double');
expect(classRef.id, 'classes|dart:core|Double');
});
Expand All @@ -76,8 +76,8 @@ void main() {
final remoteObject = await libraryPublicFinal();
final count = await inspector.loadField(remoteObject, 'myselfField');
final ref = await inspector.instanceRefFor(count);
expect(ref.kind, InstanceKind.kPlainInstance);
final classRef = ref.classRef;
expect(ref!.kind, InstanceKind.kPlainInstance);
final classRef = ref.classRef!;
expect(classRef.name, 'MyTestClass<dynamic>');
expect(
classRef.id,
Expand All @@ -87,11 +87,11 @@ void main() {

test('for closure', () async {
final remoteObject = await libraryPublicFinal();
final properties = await debugger.getProperties(remoteObject.objectId);
final properties = await debugger.getProperties(remoteObject.objectId!);
final closure =
properties.firstWhere((property) => property.name == 'closure');
final instanceRef = await inspector.instanceRefFor(closure.value);
final functionName = instanceRef.closureFunction.name;
final instanceRef = await inspector.instanceRefFor(closure.value!);
final functionName = instanceRef!.closureFunction!.name;
// Older SDKs do not contain function names
if (functionName != 'Closure') {
expect(functionName, 'someFunction');
Expand All @@ -102,40 +102,40 @@ void main() {
test('for a list', () async {
final remoteObject = await libraryPublic();
final ref = await inspector.instanceRefFor(remoteObject);
expect(ref.length, greaterThan(0));
expect(ref!.length, greaterThan(0));
expect(ref.kind, InstanceKind.kList);
expect(ref.classRef.name, 'List<String>');
expect(ref.classRef!.name, 'List<String>');
});

test('for map', () async {
final remoteObject =
await inspector.jsEvaluate(libraryVariableExpression('map'));
final ref = await inspector.instanceRefFor(remoteObject);
expect(ref.length, 2);
expect(ref!.length, 2);
expect(ref.kind, InstanceKind.kMap);
expect(ref.classRef.name, 'LinkedMap<Object, Object>');
expect(ref.classRef!.name, 'LinkedMap<Object, Object>');
});

test('for an IdentityMap', () async {
final remoteObject =
await inspector.jsEvaluate(libraryVariableExpression('identityMap'));
final ref = await inspector.instanceRefFor(remoteObject);
expect(ref.length, 2);
expect(ref!.length, 2);
expect(ref.kind, InstanceKind.kMap);
expect(ref.classRef.name, 'IdentityMap<String, int>');
expect(ref.classRef!.name, 'IdentityMap<String, int>');
});
});

group('instance', () {
test('for class object', () async {
final remoteObject = await libraryPublicFinal();
final instance = await inspector.instanceFor(remoteObject);
expect(instance.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef!;
expect(classRef, isNotNull);
expect(classRef.name, 'MyTestClass<dynamic>');
final fieldNames =
instance.fields.map((boundField) => boundField.decl.name).toList();
instance.fields!.map((boundField) => boundField.decl!.name).toList();
expect(fieldNames, [
'_privateField',
'abstractField',
Expand All @@ -146,54 +146,54 @@ void main() {
'notFinal',
'tornOff',
]);
for (var field in instance.fields) {
expect(field.decl.declaredType, isNotNull);
for (var field in instance.fields!) {
expect(field.decl!.declaredType, isNotNull);
}
});

test('for closure', () async {
final remoteObject = await libraryPublicFinal();
final properties = await debugger.getProperties(remoteObject.objectId);
final properties = await debugger.getProperties(remoteObject.objectId!);
final closure =
properties.firstWhere((property) => property.name == 'closure');
final instance = await inspector.instanceFor(closure.value);
expect(instance.kind, InstanceKind.kClosure);
expect(instance.classRef.name, 'Closure');
final instance = await inspector.instanceFor(closure.value!);
expect(instance!.kind, InstanceKind.kClosure);
expect(instance.classRef!.name, 'Closure');
});

test('for a nested class', () async {
final libraryRemoteObject = await libraryPublicFinal();
final fieldRemoteObject =
await inspector.loadField(libraryRemoteObject, 'myselfField');
final instance = await inspector.instanceFor(fieldRemoteObject);
expect(instance.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef!;
expect(classRef, isNotNull);
expect(classRef.name, 'MyTestClass<dynamic>');
});

test('for a list', () async {
final remote = await libraryPublic();
final instance = await inspector.instanceFor(remote);
expect(instance.kind, InstanceKind.kList);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kList);
final classRef = instance.classRef!;
expect(classRef, isNotNull);
expect(classRef.name, 'List<String>');
final first = instance.elements[0];
final first = instance.elements![0];
expect(first.valueAsString, 'library');
});

test('for a map', () async {
final remote =
await inspector.jsEvaluate(libraryVariableExpression('map'));
final instance = await inspector.instanceFor(remote);
expect(instance.kind, InstanceKind.kMap);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kMap);
final classRef = instance.classRef!;
expect(classRef.name, 'LinkedMap<Object, Object>');
final first = instance.associations[0].value as InstanceRef;
final first = instance.associations![0].value as InstanceRef;
expect(first.kind, InstanceKind.kList);
expect(first.length, 3);
final second = instance.associations[1].value as InstanceRef;
final second = instance.associations![1].value as InstanceRef;
expect(second.kind, InstanceKind.kString);
expect(second.valueAsString, 'something');
});
Expand All @@ -202,10 +202,10 @@ void main() {
final remote =
await inspector.jsEvaluate(libraryVariableExpression('identityMap'));
final instance = await inspector.instanceFor(remote);
expect(instance.kind, InstanceKind.kMap);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kMap);
final classRef = instance.classRef!;
expect(classRef.name, 'IdentityMap<String, int>');
final first = instance.associations[0].value;
final first = instance.associations![0].value;
expect(first.valueAsString, '1');
});

Expand All @@ -214,12 +214,12 @@ void main() {
final remote =
await inspector.jsEvaluate(libraryVariableExpression('notAList'));
final instance = await inspector.instanceFor(remote);
expect(instance.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef;
expect(instance!.kind, InstanceKind.kPlainInstance);
final classRef = instance.classRef!;
expect(classRef.name, 'NotReallyAList');
expect(instance.elements, isNull);
final field = instance.fields.first;
expect(field.decl.name, '_internal');
final field = instance.fields!.first;
expect(field.decl!.name, '_internal');
});
});
}