From eb2ebc4b2363b36ef9103c5efb6fce3cc6f544a4 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 8 Jun 2022 13:24:29 -0400 Subject: [PATCH 1/3] [tool] Check integration tests for `test` Ensures that tests in integration test files are written with `testWidgets` instead of `test`, as the latter won't correctly report failures in an integration test. See https://github.com/flutter/flutter/issues/105055 --- script/tool/CHANGELOG.md | 2 + .../tool/lib/src/drive_examples_command.dart | 30 +++++++++++++- .../test/drive_examples_command_test.dart | 41 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index adc7bfcd29ae..36d8d23eb753 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,6 +1,8 @@ ## NEXT - Supports empty custom analysis allow list files. +- `drive-examples` now validates files to ensure that they don't accidentally + use `test(...)`. ## 0.8.6 diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index 15366e17ae85..45e20c0f13cf 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -182,7 +182,16 @@ class DriveExamplesCommand extends PackageLoopingCommand { if (legacyTestFile != null) { testTargets.add(legacyTestFile); } else { - (await _getIntegrationTests(example)).forEach(testTargets.add); + for (final File testFile in await _getIntegrationTests(example)) { + // Check files for known problematic patterns. + final bool passesValidation = _validateIntegrationTest(testFile); + if (!passesValidation) { + // Report the issue, but continue with the test as the validation + // errors don't prevent running. + errors.add('${testFile.basename} failed validation'); + } + testTargets.add(testFile); + } } if (testTargets.isEmpty) { @@ -310,6 +319,25 @@ class DriveExamplesCommand extends PackageLoopingCommand { return tests; } + /// Checks [testFile] for known bad patterns in integration tests, logging + /// any issues. + /// + /// Returns true if the file passes validation without issues. + bool _validateIntegrationTest(File testFile) { + final List lines = testFile.readAsLinesSync(); + + final RegExp badTestPattern = RegExp(r'\s*test\('); + if (lines.any((String line) => line.startsWith(badTestPattern))) { + final String filename = testFile.basename; + printError( + '$filename uses "test", which will not report failures correctly. ' + 'Use testWidgets instead.'); + return false; + } + + return true; + } + /// For each file in [targets], uses /// `flutter drive --driver [driver] --target ` /// to drive [example], returning a list of any failing test targets. diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index 23318f7cd604..0b6082098ae8 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -307,6 +307,47 @@ void main() { ); }); + test('integration tests using test(...) fail validation', () async { + setMockFlutterDevicesOutput(); + final RepositoryPackage package = createFakePlugin( + 'plugin', + packagesDir, + extraFiles: [ + 'example/test_driver/integration_test.dart', + 'example/integration_test/foo_test.dart', + 'example/android/android.java', + ], + platformSupport: { + platformAndroid: const PlatformDetails(PlatformSupport.inline), + platformIOS: const PlatformDetails(PlatformSupport.inline), + }, + ); + package.directory + .childDirectory('example') + .childDirectory('integration_test') + .childFile('foo_test.dart') + .writeAsStringSync(''' + test('this is the wrong kind of test!'), () { + ... + } +'''); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--android'], + errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('foo_test.dart failed validation'), + ]), + ); + }); + test( 'driving under folder "test_driver" when targets are under "integration_test"', () async { From 9f9d2153c43bbc274778a095ce37b79932de29c5 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 8 Jun 2022 13:30:55 -0400 Subject: [PATCH 2/3] Fix violations --- .../example/integration_test/path_provider_test.dart | 2 +- .../example/integration_test/path_provider_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart b/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart index 4f5a1873bfb8..221bfea4b5ce 100644 --- a/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart +++ b/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart @@ -70,7 +70,7 @@ void main() { ]; for (final StorageDirectory? type in _allDirs) { - test('getExternalStorageDirectories (type: $type)', () async { + testWidgets('getExternalStorageDirectories (type: $type)', () async { if (Platform.isIOS) { final Future?> result = getExternalStorageDirectories(type: null); diff --git a/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart b/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart index 426b07abc7c1..5c32b1d5c3f7 100644 --- a/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart +++ b/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart @@ -61,7 +61,7 @@ void main() { ]; for (final StorageDirectory? type in _allDirs) { - test('getExternalStorageDirectories (type: $type)', () async { + testWidgets('getExternalStorageDirectories (type: $type)', () async { final PathProviderPlatform provider = PathProviderPlatform.instance; final List? directories = From c0abb4a077ae952e09fc398a58952aba77b8fe17 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 8 Jun 2022 15:46:09 -0400 Subject: [PATCH 3/3] Fix compilation --- .../example/integration_test/path_provider_test.dart | 3 ++- .../example/integration_test/path_provider_test.dart | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart b/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart index 221bfea4b5ce..6b3dd65fcb14 100644 --- a/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart +++ b/packages/path_provider/path_provider/example/integration_test/path_provider_test.dart @@ -70,7 +70,8 @@ void main() { ]; for (final StorageDirectory? type in _allDirs) { - testWidgets('getExternalStorageDirectories (type: $type)', () async { + testWidgets('getExternalStorageDirectories (type: $type)', + (WidgetTester tester) async { if (Platform.isIOS) { final Future?> result = getExternalStorageDirectories(type: null); diff --git a/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart b/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart index 5c32b1d5c3f7..0538738ade7e 100644 --- a/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart +++ b/packages/path_provider/path_provider_android/example/integration_test/path_provider_test.dart @@ -61,7 +61,8 @@ void main() { ]; for (final StorageDirectory? type in _allDirs) { - testWidgets('getExternalStorageDirectories (type: $type)', () async { + testWidgets('getExternalStorageDirectories (type: $type)', + (WidgetTester tester) async { final PathProviderPlatform provider = PathProviderPlatform.instance; final List? directories =