diff --git a/eng/tools/suppressions/src/suppressions.ts b/eng/tools/suppressions/src/suppressions.ts index 89f62f4008f7..a6f3d6433c29 100644 --- a/eng/tools/suppressions/src/suppressions.ts +++ b/eng/tools/suppressions/src/suppressions.ts @@ -1,3 +1,4 @@ +import { Stats } from "fs"; import { access, constants, lstat, readFile } from "fs/promises"; import { minimatch } from "minimatch"; import { dirname, join, resolve, sep } from "path"; @@ -37,12 +38,13 @@ const suppressionSchema = z.array( paths: paths, reason: s.reason, } as Suppression; - }) + }), ); /** - * Returns the suppressions for a tool applicable to a path. Walks up the directory tree to the first file named + * Returns the suppressions for a tool applicable to a path. Walks up the directory tree to find all files named * "suppressions.yaml", parses and validates the contents, and returns the suppressions matching the tool and path. + * Suppressions are ordered by file (closest to path is first), then within the file (closest to top is first). * * @param tool Name of tool. Matched against property "tool" in suppressions.yaml. * @param path Path to file or directory under analysis. @@ -68,17 +70,21 @@ export async function getSuppressions(tool: string, path: string): Promise { +async function findSuppressionsFiles(path: string): Promise { + const suppressionsFiles: string[] = []; + path = resolve(path); - const stats = await lstat(path); + const stats: Stats = await lstat(path); let currentDirectory: string = stats.isDirectory() ? path : dirname(path); while (true) { @@ -168,15 +178,19 @@ async function findSuppressionsYaml(path: string): Promise { try { // Throws if file cannot be read await access(suppressionsFile, constants.R_OK); - return suppressionsFile; + suppressionsFiles.push(suppressionsFile); } catch { - const parentDirectory: string = dirname(currentDirectory); - if (parentDirectory !== currentDirectory) { - currentDirectory = parentDirectory; - } else { - // Reached fs root but no "suppressions.yaml" found - return; - } + // File does not exist (or cannot be read), so skip this directory and check the parent + } + + const parentDirectory: string = dirname(currentDirectory); + if (parentDirectory !== currentDirectory) { + currentDirectory = parentDirectory; + } else { + // Reached fs root + break; } } + + return suppressionsFiles; } diff --git a/eng/tools/suppressions/test/e2e/merge/foo/bar/bar.json b/eng/tools/suppressions/test/e2e/merge/foo/bar/bar.json new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/eng/tools/suppressions/test/e2e/merge/foo/bar/suppressions.yaml b/eng/tools/suppressions/test/e2e/merge/foo/bar/suppressions.yaml new file mode 100644 index 000000000000..113dc0b7729e --- /dev/null +++ b/eng/tools/suppressions/test/e2e/merge/foo/bar/suppressions.yaml @@ -0,0 +1,11 @@ +- tool: TestTool + path: '**' + reason: bar-globstar + +- tool: TestTool + path: '*' + reason: bar-star + +- tool: TestTool + path: bar.json + reason: bar-exact diff --git a/eng/tools/suppressions/test/e2e/merge/foo/foo.json b/eng/tools/suppressions/test/e2e/merge/foo/foo.json new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/eng/tools/suppressions/test/e2e/merge/foo/suppressions.yaml b/eng/tools/suppressions/test/e2e/merge/foo/suppressions.yaml new file mode 100644 index 000000000000..cdc396f96095 --- /dev/null +++ b/eng/tools/suppressions/test/e2e/merge/foo/suppressions.yaml @@ -0,0 +1,23 @@ +- tool: TestTool + path: '**' + reason: foo-globstar + +- tool: TestTool + path: '*' + reason: foo-star + +- tool: TestTool + path: foo.json + reason: foo-exact + +- tool: TestTool + path: bar/** + reason: foo-bar-globstar + +- tool: TestTool + path: bar/* + reason: foo-bar-star + +- tool: TestTool + path: bar/bar.json + reason: foo-bar-exact diff --git a/eng/tools/suppressions/test/e2e/merge/suppressions.yaml b/eng/tools/suppressions/test/e2e/merge/suppressions.yaml new file mode 100644 index 000000000000..e664f46a770b --- /dev/null +++ b/eng/tools/suppressions/test/e2e/merge/suppressions.yaml @@ -0,0 +1,23 @@ +- tool: TestTool + path: foo/** + reason: root-foo-globstar + +- tool: TestTool + path: foo/* + reason: root-foo-star + +- tool: TestTool + path: foo/foo.json + reason: root-foo-exact + +- tool: TestTool + path: foo/bar/** + reason: root-foo-bar-globstar + +- tool: TestTool + path: foo/bar/* + reason: root-foo-bar-star + +- tool: TestTool + path: foo/bar/bar.json + reason: root-foo-bar-exact diff --git a/eng/tools/suppressions/test/suppressions.e2e.test.ts b/eng/tools/suppressions/test/suppressions.e2e.test.ts index 865dc191b74a..848dfdd842a8 100644 --- a/eng/tools/suppressions/test/suppressions.e2e.test.ts +++ b/eng/tools/suppressions/test/suppressions.e2e.test.ts @@ -2,6 +2,13 @@ import { expect, test } from "vitest"; import { Suppression, getSuppressions } from "../src/suppressions.js"; import { join } from "path"; +/** + * Returns the suppressions for a tool (default "TestTool") applicable to a path under folder "e2e". + * + * @param path Relative path to to file or directory to analyze, under folder "e2e". + * @param tool Name of tool. Matched against property "tool" in suppressions.yaml. Defaults to "TestTool". + * @returns Array of suppressions matching tool and path (may be empty). + */ async function getTestSuppressions( path: string, tool: string = "TestTool", @@ -46,3 +53,102 @@ test.concurrent("suppress foo.json, get foo.json w/ different tool", async () => ); expect(suppressions).toEqual([]); }); + +test.concurrent("merge, get bar.json", async () => { + const suppressions: Suppression[] = await getTestSuppressions( + join("merge", "foo", "bar", "bar.json"), + ); + expect(suppressions).toEqual([ + { + tool: "TestTool", + paths: ["**"], + reason: "bar-globstar", + }, + { + tool: "TestTool", + paths: ["*"], + reason: "bar-star", + }, + { + tool: "TestTool", + paths: ["bar.json"], + reason: "bar-exact", + }, + { + tool: "TestTool", + paths: ["**"], + reason: "foo-globstar", + }, + { + tool: "TestTool", + paths: ["bar/**"], + reason: "foo-bar-globstar", + }, + { + tool: "TestTool", + paths: ["bar/*"], + reason: "foo-bar-star", + }, + { + tool: "TestTool", + paths: ["bar/bar.json"], + reason: "foo-bar-exact", + }, + { + tool: "TestTool", + paths: ["foo/**"], + reason: "root-foo-globstar", + }, + { + tool: "TestTool", + paths: ["foo/bar/**"], + reason: "root-foo-bar-globstar", + }, + { + tool: "TestTool", + paths: ["foo/bar/*"], + reason: "root-foo-bar-star", + }, + { + tool: "TestTool", + paths: ["foo/bar/bar.json"], + reason: "root-foo-bar-exact", + }, + ]); +}); + +test.concurrent("merge, get foo.json", async () => { + const suppressions: Suppression[] = await getTestSuppressions(join("merge", "foo", "foo.json")); + expect(suppressions).toEqual([ + { + tool: "TestTool", + paths: ["**"], + reason: "foo-globstar", + }, + { + tool: "TestTool", + paths: ["*"], + reason: "foo-star", + }, + { + tool: "TestTool", + paths: ["foo.json"], + reason: "foo-exact", + }, + { + tool: "TestTool", + paths: ["foo/**"], + reason: "root-foo-globstar", + }, + { + tool: "TestTool", + paths: ["foo/*"], + reason: "root-foo-star", + }, + { + tool: "TestTool", + paths: ["foo/foo.json"], + reason: "root-foo-exact", + }, + ]); +});