From 7723022cdc6dfb4d5e502ec07ae8444bc9305c90 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 8 Jun 2022 17:53:09 -0400 Subject: [PATCH] [tools] Check integration tests for `test` (#5936) --- .../integration_test/path_provider_test.dart | 3 +- .../integration_test/path_provider_test.dart | 3 +- script/tool/CHANGELOG.md | 2 + .../tool/lib/src/drive_examples_command.dart | 30 +++++++++++++- .../test/drive_examples_command_test.dart | 41 +++++++++++++++++++ 5 files changed, 76 insertions(+), 3 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..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) { - test('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 426b07abc7c1..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) { - test('getExternalStorageDirectories (type: $type)', () async { + testWidgets('getExternalStorageDirectories (type: $type)', + (WidgetTester tester) async { final PathProviderPlatform provider = PathProviderPlatform.instance; final List? directories = 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 {