From cd45fc09eab50329a81312202b2645269691e9fa Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Thu, 27 May 2021 18:03:50 -0700 Subject: [PATCH] Make litetest throw on test failure instead of exit() (flutter/engine#26462) --- .../dart/isolate_name_server_test.dart | 68 ++++++++++++------- .../testing/litetest/lib/src/test_suite.dart | 8 +-- engine/src/flutter/testing/run_tests.py | 2 +- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/engine/src/flutter/testing/dart/isolate_name_server_test.dart b/engine/src/flutter/testing/dart/isolate_name_server_test.dart index 1e44bc22e9d..78cb358933d 100644 --- a/engine/src/flutter/testing/dart/isolate_name_server_test.dart +++ b/engine/src/flutter/testing/dart/isolate_name_server_test.dart @@ -3,93 +3,105 @@ // found in the LICENSE file. // @dart = 2.6 +import 'dart:async'; import 'dart:isolate'; import 'dart:ui'; import 'package:litetest/litetest.dart'; -const String kPortName = 'foobar'; const int kErrorCode = -1; const int kStartCode = 0; const int kCloseCode = 1; const int kDeletedCode = 2; -void isolateSpawnEntrypoint(SendPort port) { +class IsolateSpawnInfo { + IsolateSpawnInfo(this.sendPort, this.portName); + + final SendPort sendPort; + final String portName; +} + +void isolateSpawnEntrypoint(IsolateSpawnInfo info) { + final SendPort port = info.sendPort; + final String portName = info.portName; void sendHelper(int code, [String message = '']) { port.send([code, message]); } - final SendPort shared = IsolateNameServer.lookupPortByName(kPortName); + final SendPort shared = IsolateNameServer.lookupPortByName(portName); if (shared == null) { - sendHelper(kErrorCode, 'Could not find port: $kPortName'); + sendHelper(kErrorCode, 'Could not find port: $portName'); return; } // ack that the SendPort lookup was successful. sendHelper(kStartCode); - shared.send(kPortName); + shared.send(portName); sendHelper(kCloseCode); // We'll fail if the ReceivePort's callback is called more than once. Try to // send another message to ensure we don't crash. shared.send('garbage'); - final bool result = IsolateNameServer.removePortNameMapping(kPortName); + final bool result = IsolateNameServer.removePortNameMapping(portName); if (result) { sendHelper(kDeletedCode); } else { - sendHelper(kErrorCode, 'Was unable to remove mapping for $kPortName'); + sendHelper(kErrorCode, 'Was unable to remove mapping for $portName'); } } void main() { test('simple isolate name server', () { + const String portName = 'foobar1'; try { // Mapping for 'foobar' isn't set. Check these cases to ensure correct // negative response. - expect(IsolateNameServer.lookupPortByName(kPortName), isNull); - expect(IsolateNameServer.removePortNameMapping(kPortName), isFalse); + expect(IsolateNameServer.lookupPortByName(portName), isNull); + expect(IsolateNameServer.removePortNameMapping(portName), isFalse); // Register a SendPort. final ReceivePort receivePort = ReceivePort(); final SendPort sendPort = receivePort.sendPort; - expect(IsolateNameServer.registerPortWithName(sendPort, kPortName), isTrue); - expect(IsolateNameServer.lookupPortByName(kPortName), sendPort); + expect(IsolateNameServer.registerPortWithName(sendPort, portName), isTrue); + expect(IsolateNameServer.lookupPortByName(portName), sendPort); // Check we can't register the same name twice. final ReceivePort receivePort2 = ReceivePort(); final SendPort sendPort2 = receivePort2.sendPort; expect( - IsolateNameServer.registerPortWithName(sendPort2, kPortName), isFalse); - expect(IsolateNameServer.lookupPortByName(kPortName), sendPort); + IsolateNameServer.registerPortWithName(sendPort2, portName), isFalse); + expect(IsolateNameServer.lookupPortByName(portName), sendPort); // Remove the mapping. - expect(IsolateNameServer.removePortNameMapping(kPortName), isTrue); - expect(IsolateNameServer.lookupPortByName(kPortName), isNull); + expect(IsolateNameServer.removePortNameMapping(portName), isTrue); + expect(IsolateNameServer.lookupPortByName(portName), isNull); // Ensure registering a new port with the old name returns the new port. expect( - IsolateNameServer.registerPortWithName(sendPort2, kPortName), isTrue); - expect(IsolateNameServer.lookupPortByName(kPortName), sendPort2); + IsolateNameServer.registerPortWithName(sendPort2, portName), isTrue); + expect(IsolateNameServer.lookupPortByName(portName), sendPort2); // Close so the test runner doesn't hang. receivePort.close(); receivePort2.close(); } finally { - IsolateNameServer.removePortNameMapping(kPortName); + IsolateNameServer.removePortNameMapping(portName); } }); test('isolate name server multi-isolate', () async { + const String portName = 'foobar2'; try { // Register our send port with the name server. final ReceivePort receivePort = ReceivePort(); final SendPort sendPort = receivePort.sendPort; - expect(IsolateNameServer.registerPortWithName(sendPort, kPortName), isTrue); + expect(IsolateNameServer.registerPortWithName(sendPort, portName), isTrue); // Test driver. final ReceivePort testReceivePort = ReceivePort(); + final Completer testPortCompleter = Completer(); testReceivePort.listen(expectAsync1((dynamic response) { final int code = response[0] as int; final String message = response[1] as String; @@ -100,7 +112,7 @@ void main() { receivePort.close(); break; case kDeletedCode: - expect(IsolateNameServer.lookupPortByName(kPortName), isNull); + expect(IsolateNameServer.lookupPortByName(portName), isNull); // Test is done, close the last ReceivePort. testReceivePort.close(); break; @@ -109,17 +121,23 @@ void main() { default: throw 'UNREACHABLE'; } - }, count: 3)); + }, count: 3), onDone: testPortCompleter.complete); + final Completer portCompleter = Completer(); receivePort.listen(expectAsync1((dynamic message) { // If we don't get this message, we timeout and fail. - expect(message, kPortName); - })); + expect(message, portName); + }), onDone: portCompleter.complete); // Run the test. - await Isolate.spawn(isolateSpawnEntrypoint, testReceivePort.sendPort); + await Isolate.spawn( + isolateSpawnEntrypoint, + IsolateSpawnInfo(testReceivePort.sendPort, portName), + ); + await testPortCompleter.future; + await portCompleter.future; } finally { - IsolateNameServer.removePortNameMapping(kPortName); + IsolateNameServer.removePortNameMapping(portName); } }); } diff --git a/engine/src/flutter/testing/litetest/lib/src/test_suite.dart b/engine/src/flutter/testing/litetest/lib/src/test_suite.dart index bb5c30cf261..c39d7c0b552 100644 --- a/engine/src/flutter/testing/litetest/lib/src/test_suite.dart +++ b/engine/src/flutter/testing/litetest/lib/src/test_suite.dart @@ -6,7 +6,7 @@ import 'dart:async'; import 'dart:collection'; -import 'dart:io' show exit, stdout; +import 'dart:io' show stdout; import 'dart:isolate'; import 'test.dart'; @@ -109,10 +109,8 @@ class _DefaultLifecycle implements Lifecycle { for (final Test t in _tests!) { testsSucceeded = testsSucceeded && (t.state == TestState.succeeded); } - if (testsSucceeded) { - exit(0); - } else { - exit(1); + if (!testsSucceeded) { + throw 'A test failed'; } } } diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py index 418fa864f0a..cb3bc7a17e4 100755 --- a/engine/src/flutter/testing/run_tests.py +++ b/engine/src/flutter/testing/run_tests.py @@ -276,7 +276,7 @@ def RunDartTest(build_dir, test_packages, dart_file, verbose_dart_snapshot, mult threading = 'single-threaded' print("Running test '%s' using 'flutter_tester' (%s)" % (kernel_file_name, threading)) - forbidden_output = [] if 'unopt' in build_dir else ['[ERROR'] + forbidden_output = [] if 'unopt' in build_dir or expect_failure else ['[ERROR'] RunEngineExecutable(build_dir, 'flutter_tester', None, command_args, forbidden_output=forbidden_output, expect_failure=expect_failure)