Skip to content

Commit 59998a0

Browse files
srujzssrujzs
committed
Review comments
1 parent ce5e6bb commit 59998a0

File tree

6 files changed

+144
-101
lines changed

6 files changed

+144
-101
lines changed

‎dwds/test/common/hot_restart_common.dart‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ void runTests({
5757
await recompile(hasEdits: true);
5858
}
5959

60-
/// Wait for `expectedStrings` to be printed in the console.
61-
Future<void> expectLogs(List<String> expectedStrings) async {
60+
// Wait for `expectedStrings` to be printed to the console.
61+
Future<void> waitForLogs(List<String> expectedStrings) async {
6262
final expectations = List<String>.from(expectedStrings);
6363
final completer = Completer<void>();
6464
final subscription = context.webkitDebugger.onConsoleAPICalled.listen((e) {
@@ -70,7 +70,14 @@ void runTests({
7070
}
7171
}
7272
});
73-
await completer.future;
73+
await completer.future.timeout(
74+
const Duration(minutes: 1),
75+
onTimeout: () {
76+
throw TimeoutException(
77+
'Failed to find logs: $expectedStrings in console.',
78+
);
79+
},
80+
);
7481
await subscription.cancel();
7582
}
7683

@@ -96,9 +103,9 @@ void runTests({
96103

97104
test('can live reload changes ', () async {
98105
// A full reload should clear the state.
99-
final logExpectation = expectLogs([newString]);
106+
final logFuture = waitForLogs([newString]);
100107
await makeEditAndRecompile();
101-
await logExpectation;
108+
await logFuture;
102109
});
103110
});
104111

@@ -124,9 +131,9 @@ void runTests({
124131

125132
test('can live reload changes ', () async {
126133
// A full reload should clear the state.
127-
final logExpectation = expectLogs([newString]);
134+
final logFuture = waitForLogs([newString]);
128135
await makeEditAndRecompile();
129-
await logExpectation;
136+
await logFuture;
130137
});
131138
});
132139

@@ -153,9 +160,9 @@ void runTests({
153160

154161
test('can live reload changes ', () async {
155162
// A full reload should clear the state.
156-
final logExpectation = expectLogs([newString]);
163+
final logFuture = waitForLogs([newString]);
157164
await makeEditAndRecompile();
158-
await logExpectation;
165+
await logFuture;
159166
});
160167
});
161168
},
@@ -292,15 +299,15 @@ void runTests({
292299
),
293300
);
294301
// Main is re-invoked which shouldn't clear the state.
295-
final logExpectation = expectLogs(['$originalString $newString']);
302+
final logFuture = waitForLogs(['$originalString $newString']);
296303
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
297304
expect(
298305
await fakeClient.callServiceExtension(hotRestart!),
299306
const TypeMatcher<Success>(),
300307
);
301308

302309
await eventsDone;
303-
await logExpectation;
310+
await logFuture;
304311
});
305312

306313
test('can send events before and after hot restart', () async {
@@ -334,7 +341,7 @@ void runTests({
334341

335342
await recompile();
336343
// Main is re-invoked which shouldn't clear the state.
337-
final logExpectation = expectLogs(['$originalString $originalString']);
344+
final logFuture = waitForLogs(['$originalString $originalString']);
338345
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
339346
expect(
340347
await fakeClient.callServiceExtension(hotRestart!),
@@ -353,7 +360,7 @@ void runTests({
353360
);
354361

355362
await eventsDone;
356-
await logExpectation;
363+
await logFuture;
357364
});
358365

359366
test('can refresh the page via the fullReload service extension', () async {
@@ -372,15 +379,15 @@ void runTests({
372379
),
373380
);
374381
// Should see only the new text.
375-
final logExpectation = expectLogs([newString]);
382+
final logFuture = waitForLogs([newString]);
376383
final fullReload = context.getRegisteredServiceExtension('fullReload');
377384
expect(
378385
await fakeClient.callServiceExtension(fullReload!),
379386
isA<Success>(),
380387
);
381388

382389
await eventsDone;
383-
await logExpectation;
390+
await logFuture;
384391
});
385392

386393
test('can hot restart while paused', () async {
@@ -405,10 +412,10 @@ void runTests({
405412

406413
await makeEditAndRecompile();
407414
// Main is re-invoked which shouldn't clear the state.
408-
final logExpectation = expectLogs(['$originalString $newString']);
415+
final logFuture = waitForLogs(['$originalString $newString']);
409416
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
410417
await fakeClient.callServiceExtension(hotRestart!);
411-
await logExpectation;
418+
await logFuture;
412419

413420
vm = await client.getVM();
414421
isolateId = vm.isolates!.first.id!;
@@ -445,27 +452,25 @@ void runTests({
445452
test('can hot restart with no changes, hot restart with changes, and '
446453
'hot restart again with no changes', () async {
447454
// Empty hot restart.
448-
var logExpectation = expectLogs(['$originalString $originalString']);
455+
var logFuture = waitForLogs(['$originalString $originalString']);
449456
await recompile();
450457
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
451458
await fakeClient.callServiceExtension(hotRestart!);
452-
await logExpectation;
459+
await logFuture;
453460

454461
// Hot restart.
455-
logExpectation = expectLogs([
456-
'$originalString $originalString $newString',
457-
]);
462+
logFuture = waitForLogs(['$originalString $originalString $newString']);
458463
await makeEditAndRecompile();
459464
await fakeClient.callServiceExtension(hotRestart);
460-
await logExpectation;
465+
await logFuture;
461466

462467
// Empty hot restart.
463-
logExpectation = expectLogs([
468+
logFuture = waitForLogs([
464469
'$originalString $originalString $newString $newString',
465470
]);
466471
await recompile();
467472
await fakeClient.callServiceExtension(hotRestart);
468-
await logExpectation;
473+
await logFuture;
469474
});
470475
}, timeout: Timeout.factor(2));
471476

@@ -491,14 +496,14 @@ void runTests({
491496

492497
test('can hot restart changes ', () async {
493498
// Main is re-invoked which shouldn't clear the state.
494-
final logExpectations = expectLogs([
499+
final logFutures = waitForLogs([
495500
'$originalString $newString',
496501
// The ext.flutter.disassemble callback is invoked and waited for.
497502
'start disassemble',
498503
'end disassemble',
499504
]);
500505
await makeEditAndRecompile();
501-
await logExpectations;
506+
await logFutures;
502507
});
503508

504509
test(
@@ -547,14 +552,14 @@ void runTests({
547552

548553
test('can hot restart changes ', () async {
549554
// Main is re-invoked which shouldn't clear the state.
550-
final logExpectations = expectLogs([
555+
final logFutures = waitForLogs([
551556
'$originalString $newString',
552557
// The ext.flutter.disassemble callback is invoked and waited for.
553558
'start disassemble',
554559
'end disassemble',
555560
]);
556561
await makeEditAndRecompile();
557-
await logExpectations;
562+
await logFutures;
558563
});
559564
});
560565
},
@@ -604,7 +609,7 @@ void runTests({
604609
);
605610

606611
// Main is re-invoked which shouldn't clear the state.
607-
final logExpectation = expectLogs(['$originalString $newString']);
612+
final logFuture = waitForLogs(['$originalString $newString']);
608613
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
609614
expect(
610615
await fakeClient.callServiceExtension(hotRestart!),
@@ -617,14 +622,14 @@ void runTests({
617622
final isolateId = vm.isolates!.first.id!;
618623
await client.resume(isolateId);
619624

620-
await logExpectation;
625+
await logFuture;
621626
},
622627
);
623628

624629
test(
625630
'after page refresh, does not run app until there is a resume event',
626631
() async {
627-
final logExpectation = expectLogs([newString]);
632+
final logFuture = waitForLogs([newString]);
628633
await makeEditAndRecompile();
629634
await context.webDriver.driver.refresh();
630635

@@ -645,7 +650,7 @@ void runTests({
645650
final isolateId = vm.isolates!.first.id!;
646651
await client.resume(isolateId);
647652

648-
await logExpectation;
653+
await logFuture;
649654
},
650655
);
651656
});

‎dwds/test/common/hot_restart_correctness_common.dart‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,22 @@ void runTests({
5454
}
5555
}
5656

57-
/// Wait for `expectedString` to be printed in the console.
58-
Future<void> expectLog(String expectedString) async {
57+
// Wait for `expectedString` to be printed to the console.
58+
Future<void> waitForLog(String expectedString) async {
5959
final completer = Completer<void>();
6060
final subscription = context.webkitDebugger.onConsoleAPICalled.listen((e) {
6161
if (e.args.first.value == expectedString) {
6262
completer.complete();
6363
}
6464
});
65-
await completer.future;
65+
await completer.future.timeout(
66+
const Duration(minutes: 1),
67+
onTimeout: () {
68+
throw TimeoutException(
69+
"Failed to find log: '$expectedString' in console.",
70+
);
71+
},
72+
);
6673
await subscription.cancel();
6774
}
6875

@@ -90,14 +97,14 @@ void runTests({
9097
test('initial state prints the right log', () async {
9198
final client = context.debugConnection.vmService;
9299

93-
final logExpectation = expectLog(
100+
final logFuture = waitForLog(
94101
'ConstObject(reloadVariable: 23, ConstantEqualitySuccess)',
95102
);
96103
final vm = await client.getVM();
97104
final isolate = await client.getIsolate(vm.isolates!.first.id!);
98105
final rootLib = isolate.rootLib;
99106
await client.evaluate(isolate.id!, rootLib!.id!, 'printConst()');
100-
await logExpectation;
107+
await logFuture;
101108
});
102109

103110
test(
@@ -119,7 +126,7 @@ void runTests({
119126
),
120127
);
121128

122-
final logExpectation = expectLog(
129+
final logFuture = waitForLog(
123130
'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)',
124131
);
125132
final hotRestart = context.getRegisteredServiceExtension('hotRestart');
@@ -129,7 +136,7 @@ void runTests({
129136
);
130137

131138
await eventsDone;
132-
await logExpectation;
139+
await logFuture;
133140
},
134141
);
135142
}, timeout: Timeout.factor(2));
@@ -155,11 +162,11 @@ void runTests({
155162
});
156163

157164
test('properly compares constants after hot restart', () async {
158-
final logExpectation = expectLog(
165+
final logFuture = waitForLog(
159166
'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)',
160167
);
161168
await makeEditAndRecompile();
162-
await logExpectation;
169+
await logFuture;
163170
});
164171
});
165172

@@ -184,11 +191,11 @@ void runTests({
184191
});
185192

186193
test('properly compares constants after hot restart', () async {
187-
final logExpectation = expectLog(
194+
final logFuture = waitForLog(
188195
'ConstObject(reloadVariable: 45, ConstantEqualitySuccess)',
189196
);
190197
await makeEditAndRecompile();
191-
await logExpectation;
198+
await logFuture;
192199
});
193200
});
194201
},

‎dwds/test/fixtures/context.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,7 @@ class TestContext {
572572
// TODO(srujzs): It's possible we may want a library file with the same name
573573
// as the entry file, but this function doesn't allow that. Potentially
574574
// support that.
575-
Future<void> makeEdits(
576-
List<({String file, String originalString, String newString})> edits,
577-
) async {
575+
Future<void> makeEdits(List<Edit> edits) async {
578576
// `dart:io`'s `stat` on Windows does not have millisecond precision so we
579577
// need to make sure we wait long enough that modifications result in a
580578
// timestamp that is guaranteed to be after the previous compile.
@@ -686,3 +684,5 @@ class TestContext {
686684
return lineNumber + 1;
687685
}
688686
}
687+
688+
typedef Edit = ({String file, String originalString, String newString});

0 commit comments

Comments
 (0)