Skip to content

Commit

Permalink
Add trailing slash to test exemption dir paths (#4034)
Browse files Browse the repository at this point in the history
In `GithubWebhookSubscription._isTestExempt` we exempt a collection of directories (in both the framework and the engine) from tests by checking if the filename starts with the directory path. This adds a trailing slash to any such checks.

Since `git` only stores files (not directories), all such paths will include a trailing slash. This avoids accidentally exempting directories such as `impeller/fixtures_that_actually_ship` when we only intended to exempt `impeller/fixtures`. Alright, bad example, but perhaps someday someone would have added something more ambiguous.
  • Loading branch information
cbracken authored Nov 17, 2024
1 parent e9752dc commit 2c46ff7
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -454,19 +454,19 @@ class GithubWebhookSubscription extends SubscriptionHandler {
filename.endsWith('.md') ||
// Exempt paths.
filename.startsWith('dev/devicelab/lib/versions/gallery.dart') ||
filename.startsWith('dev/integration_tests') ||
filename.startsWith('dev/integration_tests/') ||
filename.startsWith('docs/') ||
filename.startsWith('${engineBasePath}docs/') ||
// ↓↓↓ Begin engine specific paths ↓↓↓
filename == 'DEPS' || // note: in monorepo; DEPS is still at the root.
isBuildPythonScript ||
filename.startsWith('${engineBasePath}impeller/fixtures') ||
filename.startsWith('${engineBasePath}impeller/golden_tests') ||
filename.startsWith('${engineBasePath}impeller/playground') ||
filename.startsWith('${engineBasePath}shell/platform/embedder/tests') ||
filename.startsWith('${engineBasePath}shell/platform/embedder/fixtures') ||
filename.startsWith('${engineBasePath}testing') ||
filename.startsWith('${engineBasePath}tools/clangd_check');
filename.startsWith('${engineBasePath}impeller/fixtures/') ||
filename.startsWith('${engineBasePath}impeller/golden_tests/') ||
filename.startsWith('${engineBasePath}impeller/playground/') ||
filename.startsWith('${engineBasePath}shell/platform/embedder/tests/') ||
filename.startsWith('${engineBasePath}shell/platform/embedder/fixtures/') ||
filename.startsWith('${engineBasePath}testing/') ||
filename.startsWith('${engineBasePath}tools/clangd_check/');
}

bool _isFusionEnginePath(String path) => (path.startsWith('engine/') || path == 'DEPS');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,36 @@ void main() {
);
});

test('Framework labels PRs, comment if no tests. Verify a trailing slash was added to the path prefix', () async {
const int issueNumber = 123;

tester.message = generateGithubWebhookMessage(
action: 'opened',
number: issueNumber,
);
when(pullRequestsService.listFiles(Config.flutterSlug, issueNumber)).thenAnswer(
(_) => Stream<PullRequestFile>.value(
PullRequestFile()..filename = 'testingénieux/davoir/trouvé/ce/truc',
),
);

when(issuesService.listCommentsByIssue(Config.flutterSlug, issueNumber)).thenAnswer(
(_) => Stream<IssueComment>.value(
IssueComment()..body = 'some other comment',
),
);

await tester.post(webhook);

verify(
issuesService.createComment(
Config.flutterSlug,
issueNumber,
argThat(contains(config.missingTestsPullRequestMessageValue)),
),
).called(1);
});

test('Framework labels PRs, comment if no tests including hit_test.dart file', () async {
const int issueNumber = 123;

Expand Down

0 comments on commit 2c46ff7

Please sign in to comment.