diff --git a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart index dd5d75e709..2bfb3e0f76 100644 --- a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart +++ b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart @@ -34,8 +34,6 @@ Set kNeedsTests = { Config.packagesSlug, }; -final RegExp kEngineTestRegExp = RegExp(r'(tests?|benchmarks?)\.(dart|java|mm|m|cc|sh|py)$'); - // Extentions for files that use // for single line comments. // See [_allChangesAreCodeComments] for more. @visibleForTesting @@ -319,7 +317,7 @@ class GithubWebhookSubscription extends SubscriptionHandler { } // Check to see if tests were submitted with this PR. - if (_isATest(filename)) { + if (_isAFrameworkTest(filename)) { hasTests = true; } } @@ -346,7 +344,7 @@ class GithubWebhookSubscription extends SubscriptionHandler { } } - bool _isATest(String filename) { + bool _isAFrameworkTest(String filename) { if (kNotActuallyATest.any(filename.endsWith)) { return false; } @@ -412,7 +410,7 @@ class GithubWebhookSubscription extends SubscriptionHandler { needsTests = !_allChangesAreCodeComments(file); } - if (kEngineTestRegExp.hasMatch(filename.toLowerCase())) { + if (_isAnEngineTest(filename)) { hasTests = true; } } @@ -430,6 +428,14 @@ class GithubWebhookSubscription extends SubscriptionHandler { } } + bool _isAnEngineTest(String filename) { + final RegExp engineTestRegExp = RegExp(r'(tests?|benchmarks?)\.(dart|java|mm|m|cc|sh|py)$'); + return filename.contains('IosBenchmarks') || + filename.contains('IosUnitTests') || + filename.contains('scenario_app') || + engineTestRegExp.hasMatch(filename.toLowerCase()); + } + bool _fileContainsAddedCode(PullRequestFile file) { // When null, do not assume 0 lines have been added. final int linesAdded = file.additionsCount ?? 1; diff --git a/app_dart/test/request_handlers/github/webhook_subscription_test.dart b/app_dart/test/request_handlers/github/webhook_subscription_test.dart index 7abf5fe240..3019a7fcbd 100644 --- a/app_dart/test/request_handlers/github/webhook_subscription_test.dart +++ b/app_dart/test/request_handlers/github/webhook_subscription_test.dart @@ -1558,167 +1558,63 @@ void foo() { ); }); - test('Engine labels PRs, no comment if Java tests', () async { - const int issueNumber = 123; - - tester.message = generateGithubWebhookMessage( - action: 'opened', - number: issueNumber, - slug: Config.engineSlug, - ); - - when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( - (_) => Stream.fromIterable([ - PullRequestFile()..filename = 'shell/platform/android/io/flutter/Blah.java', - PullRequestFile()..filename = 'shell/platform/android/test/io/flutter/BlahTest.java', - ]), - ); - - await tester.post(webhook); - - verifyNever( - issuesService.addLabelsToIssue(Config.engineSlug, issueNumber, any), - ); - - verifyNever( - issuesService.createComment( - Config.engineSlug, - issueNumber, - argThat(contains(config.missingTestsPullRequestMessageValue)), - ), - ); - }); - - test('Engine labels PRs, no comment if script tests', () async { - const int issueNumber = 123; - - tester.message = generateGithubWebhookMessage( - action: 'opened', - number: issueNumber, - slug: Config.engineSlug, - ); - - when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( - (_) => Stream.fromIterable([ - PullRequestFile()..filename = 'fml/blah.cc', - PullRequestFile()..filename = 'fml/testing/blah_test.sh', - ]), - ); - - await tester.post(webhook); - - verifyNever( - issuesService.createComment( - Config.engineSlug, - issueNumber, - argThat(contains(config.missingTestsPullRequestMessageValue)), - ), - ); - }); - - test('Engine labels PRs, no comment if cc tests', () async { - const int issueNumber = 123; - - tester.message = generateGithubWebhookMessage( - action: 'opened', - number: issueNumber, - slug: Config.engineSlug, - ); - - when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( - (_) => Stream.fromIterable([ - PullRequestFile()..filename = 'fml/blah.cc', - PullRequestFile()..filename = 'fml/blah_unittests.cc', - ]), - ); - - await tester.post(webhook); - - verifyNever( - issuesService.addLabelsToIssue( - Config.engineSlug, - issueNumber, - any, - ), - ); - - verifyNever( - issuesService.createComment( - Config.engineSlug, - issueNumber, - argThat(contains(config.missingTestsPullRequestMessageValue)), - ), - ); - }); - - test('Engine labels PRs, no comment if py tests', () async { - const int issueNumber = 123; - - tester.message = generateGithubWebhookMessage( - action: 'opened', - number: issueNumber, - slug: Config.engineSlug, - ); - - when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( - (_) => Stream.fromIterable([ - PullRequestFile()..filename = 'tools/font-subset/main.cc', - PullRequestFile()..filename = 'tools/font-subset/test.py', - ]), - ); - - await tester.post(webhook); - - verifyNever( - issuesService.addLabelsToIssue( - Config.engineSlug, - issueNumber, - any, - ), - ); - - verifyNever( - issuesService.createComment( - Config.engineSlug, - issueNumber, - argThat(contains(config.missingTestsPullRequestMessageValue)), - ), - ); - }); - - test('Engine labels PRs, no comment if cc benchmarks', () async { - const int issueNumber = 123; - - tester.message = generateGithubWebhookMessage( - action: 'opened', - number: issueNumber, - slug: Config.engineSlug, - ); - - when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( - (_) => Stream.fromIterable([ - PullRequestFile()..filename = 'fml/blah.cc', - PullRequestFile()..filename = 'fml/blah_benchmarks.cc', - ]), - ); + test('Engine labels PRs, no comment if tested', () async { + final List> pullRequestFileList = [ + [ + // Java tests. + 'shell/platform/android/io/flutter/Blah.java', + 'shell/platform/android/test/io/flutter/BlahTest.java', + ], + [ + // Script tests. + 'fml/blah.cc', + 'fml/testing/blah_test.sh', + ], + [ + // cc tests. + 'fml/blah.cc', + 'fml/blah_unittests.cc', + ], + [ + // cc benchmarks. + 'fml/blah.cc', + 'fml/blah_benchmarks.cc', + ], + [ + // py tests. + 'tools/font-subset/main.cc', + 'tools/font-subset/test.py', + ], + [ + // scenario app is a test. + 'scenario_app/project.pbxproj', + 'scenario_app/Info_Impeller.plist', + ], + ]; + + for (int issueNumber = 0; issueNumber < pullRequestFileList.length; issueNumber++) { + tester.message = generateGithubWebhookMessage( + action: 'opened', + number: issueNumber, + slug: Config.engineSlug, + ); - await tester.post(webhook); + when(pullRequestsService.listFiles(Config.engineSlug, issueNumber)).thenAnswer( + (_) => Stream.fromIterable( + pullRequestFileList[issueNumber].map((String filename) => PullRequestFile()..filename = filename), + ), + ); - verifyNever( - issuesService.addLabelsToIssue( - Config.engineSlug, - issueNumber, - any, - ), - ); + await tester.post(webhook); - verifyNever( - issuesService.createComment( - Config.engineSlug, - issueNumber, - argThat(contains(config.missingTestsPullRequestMessageValue)), - ), - ); + verifyNever( + issuesService.createComment( + Config.engineSlug, + issueNumber, + argThat(contains(config.missingTestsPullRequestMessageValue)), + ), + ); + } }); test('Engine labels PRs, no comments if pr is for release branches', () async {