From ca6381ad28d5837db771df5762987c1241f668b7 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 03:35:38 +0000 Subject: [PATCH 01/40] [changed-files] decouple filters --- .github/shared/src/changed-files.js | 12 ++++-------- .github/shared/test/changed-files.test.js | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 433ca87ca7ef..e8e0dabfc0a1 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -167,7 +167,7 @@ export function specification(file) { */ export function dataPlane(file) { // Folder name "data-plane" should match case for consistency across specs - return typeof file === "string" && specification(file) && file.includes("/data-plane/"); + return typeof file === "string" && file.includes("/data-plane/"); } /** @@ -176,7 +176,7 @@ export function dataPlane(file) { */ export function resourceManager(file) { // Folder name "resource-manager" should match case for consistency across specs - return typeof file === "string" && specification(file) && file.includes("/resource-manager/"); + return typeof file === "string" && file.includes("/resource-manager/"); } /** @@ -185,9 +185,7 @@ export function resourceManager(file) { */ export function example(file) { // Folder name "examples" should match case for consistency across specs - return ( - typeof file === "string" && json(file) && specification(file) && file.includes("/examples/") - ); + return typeof file === "string" && json(file) && file.includes("/examples/"); } /** @@ -209,7 +207,5 @@ export function swagger(file) { * @returns {boolean} */ export function scenario(file) { - return ( - typeof file === "string" && json(file) && specification(file) && file.includes("/scenarios/") - ); + return typeof file === "string" && json(file) && file.includes("/scenarios/"); } diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index c4908df76df2..63960c53b5e7 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -46,6 +46,11 @@ describe("changedFiles", () => { "cspell.yaml", "MixedCase.jSoN", "README.MD", + "not-spec/contosowidgetmanager/data-plane/readme.md", + "not-spec/contosowidgetmanager/resource-manager/readme.md", + "not-spec/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", + "not-spec/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", + "not-spec/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/data-plane/readme.md", "specification/contosowidgetmanager/Contoso.Management/main.tsp", "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", @@ -59,6 +64,9 @@ describe("changedFiles", () => { const expected = [ "cspell.json", "MixedCase.jSoN", + "not-spec/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", + "not-spec/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", + "not-spec/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/examples/Employees_Get.json", @@ -71,6 +79,8 @@ describe("changedFiles", () => { it("filter:readme", () => { const expected = [ "README.MD", + "not-spec/contosowidgetmanager/data-plane/readme.md", + "not-spec/contosowidgetmanager/resource-manager/readme.md", "specification/contosowidgetmanager/data-plane/readme.md", "specification/contosowidgetmanager/resource-manager/readme.md", ]; @@ -93,13 +103,18 @@ describe("changedFiles", () => { }); it("filter:data-plane", () => { - const expected = ["specification/contosowidgetmanager/data-plane/readme.md"]; + const expected = [ + "not-spec/contosowidgetmanager/data-plane/readme.md", + "specification/contosowidgetmanager/data-plane/readme.md", + ]; expect(files.filter(dataPlane)).toEqual(expected); }); it("filter:resource-manager", () => { const expected = [ + "not-spec/contosowidgetmanager/resource-manager/readme.md", + "not-spec/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/resource-manager/readme.md", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/examples/Employees_Get.json", @@ -110,6 +125,7 @@ describe("changedFiles", () => { it("filter:example", () => { const expected = [ + "not-spec/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/examples/Employees_Get.json", ]; @@ -119,6 +135,7 @@ describe("changedFiles", () => { it("filter:scenarios", () => { const expected = [ + "not-spec/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", "specification/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", ]; @@ -127,6 +144,7 @@ describe("changedFiles", () => { it("filter:swagger", () => { const expected = [ + "not-spec/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", ]; From 33152780a9ffd10a83eda83dcedc649fd2a923f7 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 04:45:42 +0000 Subject: [PATCH 02/40] [swagger] remove duplicate example() --- .github/shared/src/swagger.js | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/.github/shared/src/swagger.js b/.github/shared/src/swagger.js index 8532c6cd9787..0e204c4a613a 100644 --- a/.github/shared/src/swagger.js +++ b/.github/shared/src/swagger.js @@ -4,7 +4,7 @@ import $RefParser, { ResolverError } from "@apidevtools/json-schema-ref-parser"; import { readFile } from "fs/promises"; import { dirname, relative, resolve } from "path"; import { mapAsync } from "./array.js"; -import { includesFolder } from "./path.js"; +import { example } from "./changed-files.js"; import { SpecModelError } from "./spec-model-error.js"; /** @@ -222,26 +222,6 @@ export class Swagger { } } -// TODO: Remove duplication with changed-files.js (which currently requires paths relative to repo root) - -/** - * @param {string} [file] - * @returns {boolean} - */ -function example(file) { - // Folder name "examples" should match case for consistency across specs - return typeof file === "string" && json(file) && includesFolder(file, "examples"); -} - -/** - * @param {string} [file] - * @returns {boolean} - */ -function json(file) { - // Extension "json" with any case is a valid JSON file - return typeof file === "string" && file.toLowerCase().endsWith(".json"); -} - // API version lifecycle stages export const API_VERSION_LIFECYCLE_STAGES = Object.freeze({ PREVIEW: "preview", From bfb9bca12e886c013f59ed3a3168c90c969f2a17 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 04:50:58 +0000 Subject: [PATCH 03/40] Add specification filter --- .github/workflows/src/arm-incremental-typespec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/src/arm-incremental-typespec.js b/.github/workflows/src/arm-incremental-typespec.js index e71619e7d297..472815ca0ae2 100644 --- a/.github/workflows/src/arm-incremental-typespec.js +++ b/.github/workflows/src/arm-incremental-typespec.js @@ -9,6 +9,7 @@ import { getChangedFiles, readme, resourceManager, + specification, swagger, } from "../../shared/src/changed-files.js"; import { Readme } from "../../shared/src/readme.js"; @@ -30,7 +31,7 @@ export default async function incrementalTypeSpec({ core }) { const changedFiles = await getChangedFiles(options); // Includes swaggers, readmes, and examples - const changedRmFiles = changedFiles.filter(resourceManager); + const changedRmFiles = changedFiles.filter((f) => specification(f) && resourceManager(f)); if (changedRmFiles.length == 0) { core.info("No changes to files containing path '/resource-manager/'"); @@ -128,7 +129,7 @@ export default async function incrementalTypeSpec({ core }) { changedSpecDir, ]); - // Filter files to only include RM swagger files + // Filter files to only include RM swagger files. Should already be filtered to files under "/specification". const specRmSwaggerFilesBaseBranch = specFilesBaseBranch .split("\n") .filter((file) => resourceManager(file) && swagger(file)); From d0f7b638c6935583174a1cf37876a64c6ea01518 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 04:55:34 +0000 Subject: [PATCH 04/40] [oav-runner] De-dup example() and swagger() --- eng/tools/oav-runner/src/runner.ts | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/eng/tools/oav-runner/src/runner.ts b/eng/tools/oav-runner/src/runner.ts index a9c4e191a57f..df4c1c0d7037 100644 --- a/eng/tools/oav-runner/src/runner.ts +++ b/eng/tools/oav-runner/src/runner.ts @@ -1,12 +1,11 @@ #!/usr/bin/env node +import * as fs from "fs"; import * as oav from "oav"; import * as path from "path"; -import * as fs from "fs"; +import { example, getChangedFiles, swagger } from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, import { Swagger } from "@azure-tools/specs-shared/swagger"; -import { includesFolder } from "@azure-tools/specs-shared/path"; -import { getChangedFiles } from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, import { ReportableOavError } from "./formatting.js"; export async function preCheckFiltering( @@ -125,24 +124,6 @@ async function getFiles(rootDirectory: string, directory: string): Promise d.includes("specification" + path.sep)); } -function example(file: string): boolean { - return ( - typeof file === "string" && - file.toLowerCase().endsWith(".json") && - includesFolder(file, "examples") - ); -} - -function swagger(file: string): boolean { - return ( - typeof file === "string" && - file.toLowerCase().endsWith(".json") && - (includesFolder(file, "data-plane") || includesFolder(file, "resource-manager")) && - includesFolder(file, "specification") && - !includesFolder(file, "examples") - ); -} - export async function processFilesToSpecificationList( rootDirectory: string, files: string[], From f4f61eeb361432bf48317178333e9859e7bfadf3 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 05:31:29 +0000 Subject: [PATCH 05/40] fix filters cross-plat --- .github/shared/src/changed-files.js | 52 +++++++++++++++-------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index e8e0dabfc0a1..cf05a82b0c4a 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -1,7 +1,9 @@ // @ts-check import debug from "debug"; +import { sep } from "path"; import { simpleGit } from "simple-git"; +import { includesFolder } from "./path.js"; // Enable simple-git debug logging to improve console output debug.enable("simple-git"); @@ -132,42 +134,42 @@ export async function getChangedFilesStatuses(options = {}) { return categorizedFiles; } -// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity +// Functions suitable for passing to string[].filter(), sorted alphabetically /** * @param {string} [file] * @returns {boolean} */ -export function json(file) { - // Extension "json" with any case is a valid JSON file - return typeof file === "string" && file.toLowerCase().endsWith(".json"); +export function dataPlane(file) { + // Folder name "data-plane" should match case for consistency across specs + return typeof file === "string" && includesFolder(file, "data-plane"); } /** * @param {string} [file] * @returns {boolean} */ -export function readme(file) { - // Filename "readme.md" with any case is a valid README file - return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); +export function example(file) { + // Folder name "examples" should match case for consistency across specs + return typeof file === "string" && json(file) && includesFolder(file, "examples"); } /** * @param {string} [file] * @returns {boolean} */ -export function specification(file) { - // Folder name "specification" should match case, since it already exists in repo - return typeof file === "string" && file.startsWith("specification/"); +export function json(file) { + // Extension "json" with any case is a valid JSON file + return typeof file === "string" && file.toLowerCase().endsWith(".json"); } /** * @param {string} [file] * @returns {boolean} */ -export function dataPlane(file) { - // Folder name "data-plane" should match case for consistency across specs - return typeof file === "string" && file.includes("/data-plane/"); +export function readme(file) { + // Filename "readme.md" with any case is a valid README file + return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); } /** @@ -176,16 +178,24 @@ export function dataPlane(file) { */ export function resourceManager(file) { // Folder name "resource-manager" should match case for consistency across specs - return typeof file === "string" && file.includes("/resource-manager/"); + return typeof file === "string" && includesFolder(file, "resource-manager"); } /** * @param {string} [file] * @returns {boolean} */ -export function example(file) { - // Folder name "examples" should match case for consistency across specs - return typeof file === "string" && json(file) && file.includes("/examples/"); +export function scenario(file) { + return typeof file === "string" && json(file) && includesFolder(file, "scenarios"); +} + +/** + * @param {string} [file] + * @returns {boolean} + */ +export function specification(file) { + // Folder name "specification" should match case, since it already exists in repo + return typeof file === "string" && file.split(sep)[0] === "specification"; } /** @@ -201,11 +211,3 @@ export function swagger(file) { !scenario(file) ); } - -/** - * @param {string} [file] - * @returns {boolean} - */ -export function scenario(file) { - return typeof file === "string" && json(file) && file.includes("/scenarios/"); -} From 2a78e2b9274e2140da65dc28c8d0b3bd9f6a99f8 Mon Sep 17 00:00:00 2001 From: "Mike Harder (from Dev Box)" Date: Mon, 21 Jul 2025 22:42:59 -0700 Subject: [PATCH 06/40] specification() must normalize() --- .github/shared/src/changed-files.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index cf05a82b0c4a..61d894389b89 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -1,7 +1,7 @@ // @ts-check import debug from "debug"; -import { sep } from "path"; +import { normalize, sep } from "path"; import { simpleGit } from "simple-git"; import { includesFolder } from "./path.js"; @@ -195,7 +195,7 @@ export function scenario(file) { */ export function specification(file) { // Folder name "specification" should match case, since it already exists in repo - return typeof file === "string" && file.split(sep)[0] === "specification"; + return typeof file === "string" && normalize(file).split(sep)[0] === "specification"; } /** From 50623aea891a1800ecdd2231e05503fd123923c2 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 05:51:04 +0000 Subject: [PATCH 07/40] re-sort filters --- .github/shared/src/changed-files.js | 38 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 61d894389b89..77ea42db919e 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -134,15 +134,24 @@ export async function getChangedFilesStatuses(options = {}) { return categorizedFiles; } -// Functions suitable for passing to string[].filter(), sorted alphabetically +// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity /** * @param {string} [file] * @returns {boolean} */ -export function dataPlane(file) { - // Folder name "data-plane" should match case for consistency across specs - return typeof file === "string" && includesFolder(file, "data-plane"); +export function json(file) { + // Extension "json" with any case is a valid JSON file + return typeof file === "string" && file.toLowerCase().endsWith(".json"); +} + +/** + * @param {string} [file] + * @returns {boolean} + */ +export function readme(file) { + // Filename "readme.md" with any case is a valid README file + return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); } /** @@ -158,18 +167,18 @@ export function example(file) { * @param {string} [file] * @returns {boolean} */ -export function json(file) { - // Extension "json" with any case is a valid JSON file - return typeof file === "string" && file.toLowerCase().endsWith(".json"); +export function specification(file) { + // Folder name "specification" should match case, since it already exists in repo + return typeof file === "string" && normalize(file).split(sep)[0] === "specification"; } /** * @param {string} [file] * @returns {boolean} */ -export function readme(file) { - // Filename "readme.md" with any case is a valid README file - return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); +export function dataPlane(file) { + // Folder name "data-plane" should match case for consistency across specs + return typeof file === "string" && includesFolder(file, "data-plane"); } /** @@ -189,15 +198,6 @@ export function scenario(file) { return typeof file === "string" && json(file) && includesFolder(file, "scenarios"); } -/** - * @param {string} [file] - * @returns {boolean} - */ -export function specification(file) { - // Folder name "specification" should match case, since it already exists in repo - return typeof file === "string" && normalize(file).split(sep)[0] === "specification"; -} - /** * @param {string} [file] * @returns {boolean} From c1aa67ad917d8b201963761a37e7e4949000b1a3 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 05:51:36 +0000 Subject: [PATCH 08/40] sort --- .github/shared/src/changed-files.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 77ea42db919e..78f74037f6f6 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -190,14 +190,6 @@ export function resourceManager(file) { return typeof file === "string" && includesFolder(file, "resource-manager"); } -/** - * @param {string} [file] - * @returns {boolean} - */ -export function scenario(file) { - return typeof file === "string" && json(file) && includesFolder(file, "scenarios"); -} - /** * @param {string} [file] * @returns {boolean} @@ -211,3 +203,11 @@ export function swagger(file) { !scenario(file) ); } + +/** + * @param {string} [file] + * @returns {boolean} + */ +export function scenario(file) { + return typeof file === "string" && json(file) && includesFolder(file, "scenarios"); +} From 60e64efe39392161e6afdf391d6dfdc584501cdb Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 05:52:01 +0000 Subject: [PATCH 09/40] sort --- .github/shared/src/changed-files.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 78f74037f6f6..53c6e8633087 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -154,15 +154,6 @@ export function readme(file) { return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); } -/** - * @param {string} [file] - * @returns {boolean} - */ -export function example(file) { - // Folder name "examples" should match case for consistency across specs - return typeof file === "string" && json(file) && includesFolder(file, "examples"); -} - /** * @param {string} [file] * @returns {boolean} @@ -190,6 +181,15 @@ export function resourceManager(file) { return typeof file === "string" && includesFolder(file, "resource-manager"); } +/** + * @param {string} [file] + * @returns {boolean} + */ +export function example(file) { + // Folder name "examples" should match case for consistency across specs + return typeof file === "string" && json(file) && includesFolder(file, "examples"); +} + /** * @param {string} [file] * @returns {boolean} From e23259de5f1a3e697f506e51d8037b1b1cf3a565 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:31:57 +0000 Subject: [PATCH 10/40] specification() throw on abs paths --- .github/shared/src/changed-files.js | 33 ++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 53c6e8633087..187d4caa9659 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -1,7 +1,7 @@ // @ts-check import debug from "debug"; -import { normalize, sep } from "path"; +import { isAbsolute, normalize, sep } from "path"; import { simpleGit } from "simple-git"; import { includesFolder } from "./path.js"; @@ -136,6 +136,28 @@ export async function getChangedFilesStatuses(options = {}) { // Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity +// Functions requiring paths relative to repo root + +/** + * @param {string} [file] Path to file, **relative to repo root** + * @returns {boolean} + */ +export function specification(file) { + if (typeof file !== "string") { + return false; + } + + // Return value would be misleading (false) on absolute paths, so throw to detect misuse + if (isAbsolute(file)) { + throw new Error(`Parameter 'file' must be relative to a repo root: '${file}'`); + } + + // Folder name "specification" should match case, since it already exists in repo + return normalize(file).split(sep)[0] === "specification"; +} + +// Functions accepting both relative and absolute paths, since paths are resolve()'d before searching (when needed) + /** * @param {string} [file] * @returns {boolean} @@ -154,15 +176,6 @@ export function readme(file) { return typeof file === "string" && file.toLowerCase().endsWith("readme.md"); } -/** - * @param {string} [file] - * @returns {boolean} - */ -export function specification(file) { - // Folder name "specification" should match case, since it already exists in repo - return typeof file === "string" && normalize(file).split(sep)[0] === "specification"; -} - /** * @param {string} [file] * @returns {boolean} From 912c1e951ccc73ad1e3e2620febb13609bb36601 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:32:06 +0000 Subject: [PATCH 11/40] add tests for abs paths --- .github/shared/test/changed-files.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 63960c53b5e7..1de43c197ab3 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -8,6 +8,7 @@ vi.mock("simple-git", () => ({ }), })); +import { relative, resolve } from "path"; import * as simpleGit from "simple-git"; import { dataPlane, @@ -60,6 +61,8 @@ describe("changedFiles", () => { "specification/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", ]; + const filesResolved = files.map((f) => resolve(f)); + it("filter:json", () => { const expected = [ "cspell.json", @@ -74,6 +77,7 @@ describe("changedFiles", () => { ]; expect(files.filter(json)).toEqual(expected); + expect(filesResolved.filter(json).map((f) => relative("", f))).toEqual(expected); }); it("filter:readme", () => { @@ -86,6 +90,7 @@ describe("changedFiles", () => { ]; expect(files.filter(readme)).toEqual(expected); + expect(filesResolved.filter(readme).map((f) => relative("", f))).toEqual(expected); }); it("filter:specification", () => { @@ -100,6 +105,9 @@ describe("changedFiles", () => { ]; expect(files.filter(specification)).toEqual(expected); + expect(() => filesResolved.filter(specification)).toThrowErrorMatchingInlineSnapshot( + `[Error: Parameter 'file' must be relative to a repo root: '/home/mharder/specs-mh/.github/shared/cspell.json']`, + ); }); it("filter:data-plane", () => { @@ -109,6 +117,7 @@ describe("changedFiles", () => { ]; expect(files.filter(dataPlane)).toEqual(expected); + expect(filesResolved.filter(dataPlane).map((f) => relative("", f))).toEqual(expected); }); it("filter:resource-manager", () => { @@ -121,6 +130,7 @@ describe("changedFiles", () => { ]; expect(files.filter(resourceManager)).toEqual(expected); + expect(filesResolved.filter(resourceManager).map((f) => relative("", f))).toEqual(expected); }); it("filter:example", () => { @@ -131,6 +141,7 @@ describe("changedFiles", () => { ]; expect(files.filter(example)).toEqual(expected); + expect(filesResolved.filter(example).map((f) => relative("", f))).toEqual(expected); }); it("filter:scenarios", () => { @@ -140,6 +151,7 @@ describe("changedFiles", () => { ]; expect(files.filter(scenario)).toEqual(expected); + expect(filesResolved.filter(scenario).map((f) => relative("", f))).toEqual(expected); }); it("filter:swagger", () => { @@ -149,6 +161,7 @@ describe("changedFiles", () => { ]; expect(files.filter(swagger)).toEqual(expected); + expect(filesResolved.filter(swagger).map((f) => relative("", f))).toEqual(expected); }); describe("getChangedFilesStatuses", () => { From 5c8da503dcefbcb6518bec91cd9721df95513790 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:35:35 +0000 Subject: [PATCH 12/40] fix test error message --- .github/shared/test/changed-files.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 1de43c197ab3..1812ab9ae7ba 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -105,8 +105,8 @@ describe("changedFiles", () => { ]; expect(files.filter(specification)).toEqual(expected); - expect(() => filesResolved.filter(specification)).toThrowErrorMatchingInlineSnapshot( - `[Error: Parameter 'file' must be relative to a repo root: '/home/mharder/specs-mh/.github/shared/cspell.json']`, + expect(() => filesResolved.filter(specification)).toThrowError( + "Parameter 'file' must be relative to a repo root", ); }); From 567297ce6fa6fb8b9b7adf55b0835bdfd9755e4b Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:42:49 +0000 Subject: [PATCH 13/40] 100% codecov --- .github/shared/test/changed-files.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 1812ab9ae7ba..2933c1cf5dfd 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -108,6 +108,8 @@ describe("changedFiles", () => { expect(() => filesResolved.filter(specification)).toThrowError( "Parameter 'file' must be relative to a repo root", ); + + expect([1].filter(/** @type {(v:any)=>boolean} */ (specification))).toEqual([]); }); it("filter:data-plane", () => { From 2ca625444ba1008ae81acfc1ee488c5383905377 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:43:05 +0000 Subject: [PATCH 14/40] comment --- .github/shared/test/changed-files.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 2933c1cf5dfd..b7ea06de09ab 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -109,6 +109,7 @@ describe("changedFiles", () => { "Parameter 'file' must be relative to a repo root", ); + // For codecov expect([1].filter(/** @type {(v:any)=>boolean} */ (specification))).toEqual([]); }); From 0a726498ad606fc572329df13b9f7ae1e2762125 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 06:46:59 +0000 Subject: [PATCH 15/40] fix tests x-plat --- .github/shared/test/changed-files.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index b7ea06de09ab..997c490d727c 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -8,7 +8,7 @@ vi.mock("simple-git", () => ({ }), })); -import { relative, resolve } from "path"; +import { resolve } from "path"; import * as simpleGit from "simple-git"; import { dataPlane, @@ -77,7 +77,7 @@ describe("changedFiles", () => { ]; expect(files.filter(json)).toEqual(expected); - expect(filesResolved.filter(json).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(json)).toEqual(expected.map((f) => resolve(f))); }); it("filter:readme", () => { @@ -90,7 +90,7 @@ describe("changedFiles", () => { ]; expect(files.filter(readme)).toEqual(expected); - expect(filesResolved.filter(readme).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(readme)).toEqual(expected.map((f) => resolve(f))); }); it("filter:specification", () => { @@ -120,7 +120,7 @@ describe("changedFiles", () => { ]; expect(files.filter(dataPlane)).toEqual(expected); - expect(filesResolved.filter(dataPlane).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(dataPlane)).toEqual(expected.map((f) => resolve(f))); }); it("filter:resource-manager", () => { @@ -133,7 +133,7 @@ describe("changedFiles", () => { ]; expect(files.filter(resourceManager)).toEqual(expected); - expect(filesResolved.filter(resourceManager).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(resourceManager)).toEqual(expected.map((f) => resolve(f))); }); it("filter:example", () => { @@ -144,7 +144,7 @@ describe("changedFiles", () => { ]; expect(files.filter(example)).toEqual(expected); - expect(filesResolved.filter(example).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(example)).toEqual(expected.map((f) => resolve(f))); }); it("filter:scenarios", () => { @@ -154,7 +154,7 @@ describe("changedFiles", () => { ]; expect(files.filter(scenario)).toEqual(expected); - expect(filesResolved.filter(scenario).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(scenario)).toEqual(expected.map((f) => resolve(f))); }); it("filter:swagger", () => { @@ -164,7 +164,7 @@ describe("changedFiles", () => { ]; expect(files.filter(swagger)).toEqual(expected); - expect(filesResolved.filter(swagger).map((f) => relative("", f))).toEqual(expected); + expect(filesResolved.filter(swagger)).toEqual(expected.map((f) => resolve(f))); }); describe("getChangedFilesStatuses", () => { From 35a81b9a4ba655ed769bb113cb3317fc2d0b2998 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 19:24:50 +0000 Subject: [PATCH 16/40] remove duplicate code --- eng/tools/oav-runner/src/runner.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/eng/tools/oav-runner/src/runner.ts b/eng/tools/oav-runner/src/runner.ts index df4c1c0d7037..7cad76532c3b 100644 --- a/eng/tools/oav-runner/src/runner.ts +++ b/eng/tools/oav-runner/src/runner.ts @@ -4,7 +4,12 @@ import * as fs from "fs"; import * as oav from "oav"; import * as path from "path"; -import { example, getChangedFiles, swagger } from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, +import { + example, + getChangedFiles, + specification, + swagger, +} from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, import { Swagger } from "@azure-tools/specs-shared/swagger"; import { ReportableOavError } from "./formatting.js"; @@ -135,7 +140,7 @@ export async function processFilesToSpecificationList( // files from get-changed-files are relative to the root of the repo, // though that context is passed into this from cli arguments. for (const file of files) { - if (!file.startsWith("specification/")) { + if (!specification(file)) { continue; } From a04e02f6d4a16f106e9980b874c71a3aa04ec66f Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 19:40:09 +0000 Subject: [PATCH 17/40] Add options.paths to getChangedFiles(), remove specification filter --- .github/shared/src/changed-files.js | 33 ++++--------------- .github/shared/test/changed-files.test.js | 21 ------------ .../workflows/src/arm-incremental-typespec.js | 4 +-- 3 files changed, 8 insertions(+), 50 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 187d4caa9659..dea772f304ac 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -1,7 +1,6 @@ // @ts-check import debug from "debug"; -import { isAbsolute, normalize, sep } from "path"; import { simpleGit } from "simple-git"; import { includesFolder } from "./path.js"; @@ -11,20 +10,21 @@ debug.enable("simple-git"); /** * @param {Object} [options] * @param {string} [options.baseCommitish] Default: "HEAD^". + * @param {string[]} [options.paths] Limits the diff to the named paths. If not set, includes all paths in repo. Default: [] * @param {string} [options.cwd] Current working directory. Default: process.cwd(). * @param {string} [options.headCommitish] Default: "HEAD". * @param {import('./logger.js').ILogger} [options.logger] - * @returns {Promise} List of changed files, using posix paths, relative to options.cwd. Example: ["specification/foo/Microsoft.Foo/main.tsp"]. + * @returns {Promise} List of changed files, using posix paths, relative to repo root. Example: ["specification/foo/Microsoft.Foo/main.tsp"]. */ export async function getChangedFiles(options = {}) { - const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger } = options; + const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger, paths = [] } = options; // TODO: If we need to filter based on status, instead of passing an argument to `--diff-filter, // consider using "--name-status" instead of "--name-only", and return an array of objects like // { name: "/foo/baz.js", status: Status.Renamed, previousName: "/foo/bar.js"}. // Then add filter functions to filter based on status. This is more flexible and lets consumers // filter based on status with a single call to `git diff`. - const result = await simpleGit(cwd).diff(["--name-only", baseCommitish, headCommitish]); + const result = await simpleGit(cwd).diff(["--name-only", baseCommitish, headCommitish, ...paths]); const files = result.trim().split("\n"); @@ -134,29 +134,8 @@ export async function getChangedFilesStatuses(options = {}) { return categorizedFiles; } -// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity - -// Functions requiring paths relative to repo root - -/** - * @param {string} [file] Path to file, **relative to repo root** - * @returns {boolean} - */ -export function specification(file) { - if (typeof file !== "string") { - return false; - } - - // Return value would be misleading (false) on absolute paths, so throw to detect misuse - if (isAbsolute(file)) { - throw new Error(`Parameter 'file' must be relative to a repo root: '${file}'`); - } - - // Folder name "specification" should match case, since it already exists in repo - return normalize(file).split(sep)[0] === "specification"; -} - -// Functions accepting both relative and absolute paths, since paths are resolve()'d before searching (when needed) +// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity. +// Functions accept both relative and absolute paths, since paths are resolve()'d before searching (when needed). /** * @param {string} [file] diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 997c490d727c..c9e1cc9aff56 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -19,7 +19,6 @@ import { readme, resourceManager, scenario, - specification, swagger, } from "../src/changed-files.js"; import { debugLogger } from "../src/logger.js"; @@ -93,26 +92,6 @@ describe("changedFiles", () => { expect(filesResolved.filter(readme)).toEqual(expected.map((f) => resolve(f))); }); - it("filter:specification", () => { - const expected = [ - "specification/contosowidgetmanager/data-plane/readme.md", - "specification/contosowidgetmanager/Contoso.Management/main.tsp", - "specification/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", - "specification/contosowidgetmanager/resource-manager/readme.md", - "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", - "specification/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/examples/Employees_Get.json", - "specification/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", - ]; - - expect(files.filter(specification)).toEqual(expected); - expect(() => filesResolved.filter(specification)).toThrowError( - "Parameter 'file' must be relative to a repo root", - ); - - // For codecov - expect([1].filter(/** @type {(v:any)=>boolean} */ (specification))).toEqual([]); - }); - it("filter:data-plane", () => { const expected = [ "not-spec/contosowidgetmanager/data-plane/readme.md", diff --git a/.github/workflows/src/arm-incremental-typespec.js b/.github/workflows/src/arm-incremental-typespec.js index 472815ca0ae2..4af4394d802b 100644 --- a/.github/workflows/src/arm-incremental-typespec.js +++ b/.github/workflows/src/arm-incremental-typespec.js @@ -9,7 +9,6 @@ import { getChangedFiles, readme, resourceManager, - specification, swagger, } from "../../shared/src/changed-files.js"; import { Readme } from "../../shared/src/readme.js"; @@ -25,13 +24,14 @@ debug.enable("simple-git"); export default async function incrementalTypeSpec({ core }) { const options = { cwd: process.env.GITHUB_WORKSPACE, + paths: ["specification"], logger: new CoreLogger(core), }; const changedFiles = await getChangedFiles(options); // Includes swaggers, readmes, and examples - const changedRmFiles = changedFiles.filter((f) => specification(f) && resourceManager(f)); + const changedRmFiles = changedFiles.filter(resourceManager); if (changedRmFiles.length == 0) { core.info("No changes to files containing path '/resource-manager/'"); From 559b58f4fb7e8fe744ba8dc9f89fdb6e9991dc67 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 19:43:40 +0000 Subject: [PATCH 18/40] comment --- .github/workflows/src/arm-incremental-typespec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/src/arm-incremental-typespec.js b/.github/workflows/src/arm-incremental-typespec.js index 4af4394d802b..14cd9173090a 100644 --- a/.github/workflows/src/arm-incremental-typespec.js +++ b/.github/workflows/src/arm-incremental-typespec.js @@ -129,7 +129,7 @@ export default async function incrementalTypeSpec({ core }) { changedSpecDir, ]); - // Filter files to only include RM swagger files. Should already be filtered to files under "/specification". + // Filter files to only include RM swagger files const specRmSwaggerFilesBaseBranch = specFilesBaseBranch .split("\n") .filter((file) => resourceManager(file) && swagger(file)); From 09fb3dca11938a8571e6032f1271f57b579722e6 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:41:38 +0000 Subject: [PATCH 19/40] Add paths param to getChangedFilesStatuses() --- .github/shared/src/changed-files.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index e8bcd4d77cac..56651af1f8ed 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -10,10 +10,10 @@ debug.enable("simple-git"); /** * @param {Object} [options] * @param {string} [options.baseCommitish] Default: "HEAD^". - * @param {string[]} [options.paths] Limits the diff to the named paths. If not set, includes all paths in repo. Default: [] * @param {string} [options.cwd] Current working directory. Default: process.cwd(). * @param {string} [options.headCommitish] Default: "HEAD". * @param {import('./logger.js').ILogger} [options.logger] + * @param {string[]} [options.paths] Limits the diff to the named paths. If not set, includes all paths in repo. Default: [] * @returns {Promise} List of changed files, using posix paths, relative to repo root. Example: ["specification/foo/Microsoft.Foo/main.tsp"]. */ export async function getChangedFiles(options = {}) { @@ -43,11 +43,17 @@ export async function getChangedFiles(options = {}) { * @param {string} [options.cwd] Current working directory. Default: process.cwd(). * @param {string} [options.headCommitish] Default: "HEAD". * @param {import('./logger.js').ILogger} [options.logger] + * @param {string[]} [options.paths] Limits the diff to the named paths. If not set, includes all paths in repo. Default: [] * @returns {Promise<{additions: string[], modifications: string[], deletions: string[], renames: {from: string, to: string}[], total: number}>} */ export async function getChangedFilesStatuses(options = {}) { - const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger } = options; - const result = await simpleGit(cwd).diff(["--name-status", baseCommitish, headCommitish]); + const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger, paths = [] } = options; + const result = await simpleGit(cwd).diff([ + "--name-status", + baseCommitish, + headCommitish, + ...paths, + ]); const categorizedFiles = { additions: /** @type {string[]} */ ([]), From 01933286ca18674b4b31614a63e04264d0e75ae6 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:42:06 +0000 Subject: [PATCH 20/40] [lint-diff] Pass "specification" to getChangedFiles() --- .github/workflows/lintdiff-code.yaml | 2 +- eng/tools/lint-diff/src/processChanges.ts | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.github/workflows/lintdiff-code.yaml b/.github/workflows/lintdiff-code.yaml index e8307a9ad6d2..11e0e9c187df 100644 --- a/.github/workflows/lintdiff-code.yaml +++ b/.github/workflows/lintdiff-code.yaml @@ -42,7 +42,7 @@ jobs: const { join } = await import('path'); // TODO: Logger - const changedFiles = await getChangedFiles({ cwd: 'after'}); + const changedFiles = await getChangedFiles({ cwd: 'after', paths: ['specification'] }); console.log('Changed files:', changedFiles); const filePath = join(process.cwd(), 'changed-files.txt'); diff --git a/eng/tools/lint-diff/src/processChanges.ts b/eng/tools/lint-diff/src/processChanges.ts index c57bec9648b2..706b503c29cf 100644 --- a/eng/tools/lint-diff/src/processChanges.ts +++ b/eng/tools/lint-diff/src/processChanges.ts @@ -1,13 +1,13 @@ -import { join, relative, resolve, sep } from "path"; -import { readFile } from "fs/promises"; -import { pathExists } from "./util.js"; -import { specification, readme, swagger } from "@azure-tools/specs-shared/changed-files"; +import { readme, swagger } from "@azure-tools/specs-shared/changed-files"; import { SpecModel } from "@azure-tools/specs-shared/spec-model"; -import { ReadmeAffectedTags } from "./lintdiff-types.js"; import deepEqual from "deep-eql"; +import { readFile } from "fs/promises"; +import { join, relative, resolve, sep } from "path"; +import { ReadmeAffectedTags } from "./lintdiff-types.js"; +import { pathExists } from "./util.js"; -import { deduplicateTags } from "./markdown-utils.js"; import $RefParser from "@apidevtools/json-schema-ref-parser"; +import { deduplicateTags } from "./markdown-utils.js"; export async function getRunList( beforePath: string, @@ -19,12 +19,9 @@ export async function getRunList( // Read changed files, exclude any files that should be ignored const ignoreFilesWith = ["/examples/", "/quickstart-templates/", "/scenarios/"]; - const changedSpecFiles = (await readFileList(changedFilesPath)).filter((file) => { - // File is in specification/ folder - if (!specification(file)) { - return false; - } + // Changed files should already be filtered to the top-level "specification" folder + const changedSpecFiles = (await readFileList(changedFilesPath)).filter((file) => { // File is not ignored for (const ignore of ignoreFilesWith) { if (file.includes(ignore)) { From dafb9fe72e3cd4d7703dd50ef861eb77c35ef7e2 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:42:34 +0000 Subject: [PATCH 21/40] [oav-runner] Pass "specification" to getChangedFiles() --- eng/tools/oav-runner/src/runner.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/eng/tools/oav-runner/src/runner.ts b/eng/tools/oav-runner/src/runner.ts index 7cad76532c3b..65286af5887c 100644 --- a/eng/tools/oav-runner/src/runner.ts +++ b/eng/tools/oav-runner/src/runner.ts @@ -4,12 +4,7 @@ import * as fs from "fs"; import * as oav from "oav"; import * as path from "path"; -import { - example, - getChangedFiles, - specification, - swagger, -} from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, +import { example, getChangedFiles, swagger } from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, import { Swagger } from "@azure-tools/specs-shared/swagger"; import { ReportableOavError } from "./formatting.js"; @@ -17,7 +12,8 @@ export async function preCheckFiltering( rootDirectory: string, fileList?: string[], ): Promise { - const changedFiles = fileList ?? (await getChangedFiles({ cwd: rootDirectory })); + const changedFiles = + fileList ?? (await getChangedFiles({ cwd: rootDirectory, paths: ["specification"] })); const swaggerFiles = await processFilesToSpecificationList(rootDirectory, changedFiles); @@ -140,10 +136,6 @@ export async function processFilesToSpecificationList( // files from get-changed-files are relative to the root of the repo, // though that context is passed into this from cli arguments. for (const file of files) { - if (!specification(file)) { - continue; - } - const absoluteFilePath = path.join(rootDirectory, file); // if the file is an example, we need to find the swagger file that references it From e9c5c5646d5bd73cc2467adc731619d3565d83a9 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:43:04 +0000 Subject: [PATCH 22/40] [openapi-diff-runner] Pass "specification" to getChangedFiles() --- eng/tools/openapi-diff-runner/src/command-helpers.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/eng/tools/openapi-diff-runner/src/command-helpers.ts b/eng/tools/openapi-diff-runner/src/command-helpers.ts index d3d45c17d78e..b913b72f300d 100644 --- a/eng/tools/openapi-diff-runner/src/command-helpers.ts +++ b/eng/tools/openapi-diff-runner/src/command-helpers.ts @@ -1,17 +1,17 @@ +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { getChangedFilesStatuses, swagger } from "@azure-tools/specs-shared/changed-files"; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; -import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; +import { logError, LogLevel, logMessage, setOutput } from "./log.js"; import { + BreakingChangeReviewRequiredLabel, BreakingChangesCheckType, Context, - BreakingChangeReviewRequiredLabel, VersioningReviewRequiredLabel, } from "./types/breaking-change.js"; import { ResultMessageRecord } from "./types/message.js"; import { createOadMessageProcessor } from "./utils/oad-message-processor.js"; import { createPullRequestProperties } from "./utils/pull-request.js"; -import { getChangedFilesStatuses, swagger } from "@azure-tools/specs-shared/changed-files"; -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { logError, LogLevel, logMessage, setOutput } from "./log.js"; /** * Interface for parsed CLI arguments @@ -133,6 +133,7 @@ export async function getSwaggerDiffs( baseCommitish: options.baseCommitish, cwd: options.cwd, headCommitish: options.headCommitish, + paths: ["specification"], }); // Filter each array to only include Swagger files using the swagger filter from changed-files.js From fce1a3a6f3402025218bb3fc3167c84604f0bf3e Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:43:16 +0000 Subject: [PATCH 23/40] [sdk-suppressions] Pass "specification" to getChangedFiles() --- eng/tools/sdk-suppressions/src/common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eng/tools/sdk-suppressions/src/common.ts b/eng/tools/sdk-suppressions/src/common.ts index 8fb57b0d62eb..12818c77bf1b 100644 --- a/eng/tools/sdk-suppressions/src/common.ts +++ b/eng/tools/sdk-suppressions/src/common.ts @@ -1,12 +1,12 @@ -import { parse as yamlParse } from "yaml"; import { getChangedFiles } from "@azure-tools/specs-shared/changed-files"; +import { parse as yamlParse } from "yaml"; /** * @returns {string[]} * @description get the changed files in the current PR */ export async function getSDKSuppressionsChangedFiles() { - const changedFiles = await getChangedFiles(); + const changedFiles = await getChangedFiles({ paths: ["specification"] }); const sdkSuppressionsFiles = changedFiles.filter((file) => file.endsWith("sdk-suppressions.yaml"), ); From d86f20f39296266a7c5467eddcbd4fac6eeb85ee Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:43:48 +0000 Subject: [PATCH 24/40] [summarize-impact] Add comment --- eng/tools/summarize-impact/src/cli.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/eng/tools/summarize-impact/src/cli.ts b/eng/tools/summarize-impact/src/cli.ts index 75641b3b6093..bd08cc302a63 100644 --- a/eng/tools/summarize-impact/src/cli.ts +++ b/eng/tools/summarize-impact/src/cli.ts @@ -1,15 +1,15 @@ #!/usr/bin/env node -import { evaluateImpact } from "./impact.js"; import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; import { setOutput } from "@azure-tools/specs-shared/error-reporting"; +import { evaluateImpact } from "./impact.js"; -import { resolve, join } from "path"; +import { getRootFolder } from "@azure-tools/specs-shared/simple-git"; import fs from "fs"; import { parseArgs, ParseArgsConfig } from "node:util"; +import { join, resolve } from "path"; import { LabelContext } from "./labelling-types.js"; import { PRContext } from "./PRContext.js"; -import { getRootFolder } from "@azure-tools/specs-shared/simple-git"; export async function getRoot(inputPath: string): Promise { try { @@ -92,7 +92,10 @@ export async function main() { const targetDirectory = opts.targetDirectory as string; const sourceGitRoot = await getRoot(sourceDirectory); const targetGitRoot = await getRoot(targetDirectory); + + // TODO: Should this be scoped to the "specification" folder? const fileList = await getChangedFilesStatuses({ cwd: sourceGitRoot }); + const sha = opts.sha as string; const sourceBranch = opts.sourceBranch as string; const targetBranch = opts.targetBranch as string; From 7d9b70449d57d1f9e09739643f7acb9fb0efebd6 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:44:43 +0000 Subject: [PATCH 25/40] comments --- .github/shared/src/changed-files.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 56651af1f8ed..530d7644d46b 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -140,8 +140,8 @@ export async function getChangedFilesStatuses(options = {}) { return categorizedFiles; } -// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity. -// Functions accept both relative and absolute paths, since paths are resolve()'d before searching (when needed). +// Functions suitable for passing to string[].filter(), ordered roughly in order of increasing specificity +// Functions accept both relative and absolute paths, since paths are resolve()'d before searching (when needed) /** * @param {string} [file] From e9ba4fa145c15afe4d1365ba49fa9facc86f39fd Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:46:00 +0000 Subject: [PATCH 26/40] revert imports --- eng/tools/lint-diff/src/processChanges.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/eng/tools/lint-diff/src/processChanges.ts b/eng/tools/lint-diff/src/processChanges.ts index 706b503c29cf..6d56430f91e3 100644 --- a/eng/tools/lint-diff/src/processChanges.ts +++ b/eng/tools/lint-diff/src/processChanges.ts @@ -1,13 +1,13 @@ +import { join, relative, resolve, sep } from "path"; +import { readFile } from "fs/promises"; +import { pathExists } from "./util.js"; import { readme, swagger } from "@azure-tools/specs-shared/changed-files"; import { SpecModel } from "@azure-tools/specs-shared/spec-model"; -import deepEqual from "deep-eql"; -import { readFile } from "fs/promises"; -import { join, relative, resolve, sep } from "path"; import { ReadmeAffectedTags } from "./lintdiff-types.js"; -import { pathExists } from "./util.js"; +import deepEqual from "deep-eql"; -import $RefParser from "@apidevtools/json-schema-ref-parser"; import { deduplicateTags } from "./markdown-utils.js"; +import $RefParser from "@apidevtools/json-schema-ref-parser"; export async function getRunList( beforePath: string, From 8e3f33eb860cba7b54cdda1254dd0e2d3b9bb08e Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:46:48 +0000 Subject: [PATCH 27/40] comment --- eng/tools/lint-diff/src/processChanges.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/tools/lint-diff/src/processChanges.ts b/eng/tools/lint-diff/src/processChanges.ts index 6d56430f91e3..447c06f8ba4c 100644 --- a/eng/tools/lint-diff/src/processChanges.ts +++ b/eng/tools/lint-diff/src/processChanges.ts @@ -20,7 +20,7 @@ export async function getRunList( // Read changed files, exclude any files that should be ignored const ignoreFilesWith = ["/examples/", "/quickstart-templates/", "/scenarios/"]; - // Changed files should already be filtered to the top-level "specification" folder + // Changed files should already be filtered to the top-level "specification" folder (see lintdiff-code.yaml) const changedSpecFiles = (await readFileList(changedFilesPath)).filter((file) => { // File is not ignored for (const ignore of ignoreFilesWith) { From 48c2de5297e64bcebd51f803f6c91fecfaf5038a Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:47:16 +0000 Subject: [PATCH 28/40] revert imports --- eng/tools/oav-runner/src/runner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/tools/oav-runner/src/runner.ts b/eng/tools/oav-runner/src/runner.ts index 65286af5887c..1e396bc6d309 100644 --- a/eng/tools/oav-runner/src/runner.ts +++ b/eng/tools/oav-runner/src/runner.ts @@ -1,8 +1,8 @@ #!/usr/bin/env node -import * as fs from "fs"; import * as oav from "oav"; import * as path from "path"; +import * as fs from "fs"; import { example, getChangedFiles, swagger } from "@azure-tools/specs-shared/changed-files"; //getChangedFiles, import { Swagger } from "@azure-tools/specs-shared/swagger"; From c9a53dbe2603ae7055d64e49cb076e3b9863e9e4 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:48:07 +0000 Subject: [PATCH 29/40] revert imports --- eng/tools/openapi-diff-runner/src/command-helpers.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/eng/tools/openapi-diff-runner/src/command-helpers.ts b/eng/tools/openapi-diff-runner/src/command-helpers.ts index b913b72f300d..fbad6d1a7362 100644 --- a/eng/tools/openapi-diff-runner/src/command-helpers.ts +++ b/eng/tools/openapi-diff-runner/src/command-helpers.ts @@ -1,17 +1,17 @@ -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { getChangedFilesStatuses, swagger } from "@azure-tools/specs-shared/changed-files"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; -import { logError, LogLevel, logMessage, setOutput } from "./log.js"; +import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import { - BreakingChangeReviewRequiredLabel, BreakingChangesCheckType, Context, + BreakingChangeReviewRequiredLabel, VersioningReviewRequiredLabel, } from "./types/breaking-change.js"; import { ResultMessageRecord } from "./types/message.js"; import { createOadMessageProcessor } from "./utils/oad-message-processor.js"; import { createPullRequestProperties } from "./utils/pull-request.js"; +import { getChangedFilesStatuses, swagger } from "@azure-tools/specs-shared/changed-files"; +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { logError, LogLevel, logMessage, setOutput } from "./log.js"; /** * Interface for parsed CLI arguments From dbb55dfda1a990da575724cc011916b9c0ff362e Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:49:07 +0000 Subject: [PATCH 30/40] revert imports --- eng/tools/sdk-suppressions/src/common.ts | 2 +- eng/tools/summarize-impact/src/cli.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/eng/tools/sdk-suppressions/src/common.ts b/eng/tools/sdk-suppressions/src/common.ts index 12818c77bf1b..6d6120fa0e2f 100644 --- a/eng/tools/sdk-suppressions/src/common.ts +++ b/eng/tools/sdk-suppressions/src/common.ts @@ -1,5 +1,5 @@ -import { getChangedFiles } from "@azure-tools/specs-shared/changed-files"; import { parse as yamlParse } from "yaml"; +import { getChangedFiles } from "@azure-tools/specs-shared/changed-files"; /** * @returns {string[]} diff --git a/eng/tools/summarize-impact/src/cli.ts b/eng/tools/summarize-impact/src/cli.ts index bd08cc302a63..243e91ab2e6a 100644 --- a/eng/tools/summarize-impact/src/cli.ts +++ b/eng/tools/summarize-impact/src/cli.ts @@ -1,15 +1,15 @@ #!/usr/bin/env node +import { evaluateImpact } from "./impact.js"; import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; import { setOutput } from "@azure-tools/specs-shared/error-reporting"; -import { evaluateImpact } from "./impact.js"; -import { getRootFolder } from "@azure-tools/specs-shared/simple-git"; +import { resolve, join } from "path"; import fs from "fs"; import { parseArgs, ParseArgsConfig } from "node:util"; -import { join, resolve } from "path"; import { LabelContext } from "./labelling-types.js"; import { PRContext } from "./PRContext.js"; +import { getRootFolder } from "@azure-tools/specs-shared/simple-git"; export async function getRoot(inputPath: string): Promise { try { From af8532504b7aa52a91e4a50165aa43cda183669d Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:57:34 +0000 Subject: [PATCH 31/40] remove spec() from typespec() --- .github/shared/src/changed-files.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index 530d7644d46b..fa738205fec7 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -195,8 +195,7 @@ export function example(file) { export function typespec(file) { return ( typeof file === "string" && - (file.toLowerCase().endsWith(".tsp") || file.toLowerCase().endsWith("tspconfig.yaml")) && - specification(file) + (file.toLowerCase().endsWith(".tsp") || file.toLowerCase().endsWith("tspconfig.yaml")) ); } From bc8bd9ec846864552ee4ada2e4777c29cb96d0b8 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 20:59:12 +0000 Subject: [PATCH 32/40] [summarize-impact] Remove dep on specification() filter --- eng/tools/summarize-impact/src/PRContext.ts | 4 ++-- eng/tools/summarize-impact/src/cli.ts | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/eng/tools/summarize-impact/src/PRContext.ts b/eng/tools/summarize-impact/src/PRContext.ts index 1038892c9724..b2dbab5df102 100644 --- a/eng/tools/summarize-impact/src/PRContext.ts +++ b/eng/tools/summarize-impact/src/PRContext.ts @@ -4,6 +4,7 @@ import * as fs from "fs"; import { parseMarkdown } from "@azure-tools/openapi-tools-common"; import * as amd from "@azure/openapi-markdown"; +import { includesFolder } from "@azure-tools/specs-shared/path"; import { SpecModel } from "@azure-tools/specs-shared/spec-model"; import { Readme } from "@azure-tools/specs-shared/readme"; import { @@ -11,7 +12,6 @@ import { typespec, example, readme, - specification, } from "@azure-tools/specs-shared/changed-files"; import { LabelContext } from "./labelling-types.js"; @@ -168,7 +168,7 @@ export class PRContext { if (visitedFolder.has(dir)) { return; } - while (specification(dir)) { + while (includesFolder(dir, "specification")) { if (visitedFolder.has(dir)) { break; } diff --git a/eng/tools/summarize-impact/src/cli.ts b/eng/tools/summarize-impact/src/cli.ts index 243e91ab2e6a..c939311a3c42 100644 --- a/eng/tools/summarize-impact/src/cli.ts +++ b/eng/tools/summarize-impact/src/cli.ts @@ -92,10 +92,7 @@ export async function main() { const targetDirectory = opts.targetDirectory as string; const sourceGitRoot = await getRoot(sourceDirectory); const targetGitRoot = await getRoot(targetDirectory); - - // TODO: Should this be scoped to the "specification" folder? - const fileList = await getChangedFilesStatuses({ cwd: sourceGitRoot }); - + const fileList = await getChangedFilesStatuses({ cwd: sourceGitRoot, paths: ["specification"] }); const sha = opts.sha as string; const sourceBranch = opts.sourceBranch as string; const targetBranch = opts.targetBranch as string; From d8431c36cf862c4c01a69b36847a1e9456ca3cec Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:30:52 +0000 Subject: [PATCH 33/40] [changed-files] Use "--" to separate paths from revisions --- .github/shared/src/changed-files.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/shared/src/changed-files.js b/.github/shared/src/changed-files.js index fa738205fec7..5641a95e64ed 100644 --- a/.github/shared/src/changed-files.js +++ b/.github/shared/src/changed-files.js @@ -19,6 +19,11 @@ debug.enable("simple-git"); export async function getChangedFiles(options = {}) { const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger, paths = [] } = options; + if (paths.length > 0) { + // Use "--" to separate paths from revisions + paths.unshift("--"); + } + // TODO: If we need to filter based on status, instead of passing an argument to `--diff-filter, // consider using "--name-status" instead of "--name-only", and return an array of objects like // { name: "/foo/baz.js", status: Status.Renamed, previousName: "/foo/bar.js"}. @@ -48,6 +53,12 @@ export async function getChangedFiles(options = {}) { */ export async function getChangedFilesStatuses(options = {}) { const { baseCommitish = "HEAD^", cwd, headCommitish = "HEAD", logger, paths = [] } = options; + + if (paths.length > 0) { + // Use "--" to separate paths from revisions + paths.unshift("--"); + } + const result = await simpleGit(cwd).diff([ "--name-status", baseCommitish, From 06772daf33cd82e99a37f5fbf4d2e682f3463959 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:44:15 +0000 Subject: [PATCH 34/40] fix test --- .../test/command-helpers.test.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index da40bb8c4819..9534c5cb3dd5 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -1,30 +1,30 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { - createContextFromParsedArgs, BreakingChangeLabelsToBeAdded, - getSwaggerDiffs, buildPrInfo, changeBaseBranch, - logFullOadMessagesList, - createDummySwagger, cleanDummySwagger, - isSameVersionBreakingType, + createContextFromParsedArgs, + createDummySwagger, getCreatedDummySwaggerCount, + getSwaggerDiffs, + isSameVersionBreakingType, + logFullOadMessagesList, outputBreakingChangeLabelVariables, type ParsedCliArguments, } from "../src/command-helpers.js"; +import { LogLevel } from "../src/log.js"; import { - Context, BreakingChangeReviewRequiredLabel, + Context, VersioningReviewRequiredLabel, } from "../src/types/breaking-change.js"; import { ResultMessageRecord } from "../src/types/message.js"; -import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { LogLevel } from "../src/log.js"; // Test constants const TEST_CONSTANTS = { @@ -451,6 +451,7 @@ describe("command-helpers", () => { baseCommitish: undefined, cwd: undefined, headCommitish: undefined, + paths: ["specification"], }); }); From ed9a32dd283c361c9d3968b209e4b9d78b393e8d Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:44:42 +0000 Subject: [PATCH 35/40] fix test --- eng/tools/openapi-diff-runner/test/command-helpers.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index 9534c5cb3dd5..3aa1c0a8e480 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -370,6 +370,7 @@ describe("command-helpers", () => { baseCommitish: TEST_CONSTANTS.BRANCHES.MAIN, cwd: TEST_CONSTANTS.PATHS.TEST_PATH, headCommitish: TEST_CONSTANTS.COMMITS.HEAD, + path: ["specification"], }); }); From 17cef0735ff666a519db2618fb1b71cd1877ba2a Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:48:04 +0000 Subject: [PATCH 36/40] Add tests for filter "typespec" --- .github/shared/test/changed-files.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/shared/test/changed-files.test.js b/.github/shared/test/changed-files.test.js index 8cb4ebec0ccc..92bd84b827db 100644 --- a/.github/shared/test/changed-files.test.js +++ b/.github/shared/test/changed-files.test.js @@ -49,6 +49,8 @@ describe("changedFiles", () => { "README.MD", "not-spec/contosowidgetmanager/data-plane/readme.md", "not-spec/contosowidgetmanager/resource-manager/readme.md", + "not-spec/contosowidgetmanager/Contoso.Management/main.tsp", + "not-spec/contosowidgetmanager/Contoso.Management/tspconfig.yaml", "not-spec/contosowidgetmanager/Contoso.Management/examples/2021-11-01/Employees_Get.json", "not-spec/contosowidgetmanager/Contoso.Management/scenarios/2021-11-01/Employees_Get.json", "not-spec/contosowidgetmanager/resource-manager/Microsoft.Contoso/stable/2021-11-01/contoso.json", @@ -96,6 +98,8 @@ describe("changedFiles", () => { it("filter:typespec", () => { const expected = [ + "not-spec/contosowidgetmanager/Contoso.Management/main.tsp", + "not-spec/contosowidgetmanager/Contoso.Management/tspconfig.yaml", "specification/contosowidgetmanager/Contoso.Management/main.tsp", "specification/contosowidgetmanager/Contoso.Management/tspconfig.yaml", ]; From aee51f4b67c1ed3b65324837ce3085c00c8d58e5 Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:50:21 +0000 Subject: [PATCH 37/40] revert imports --- .../test/command-helpers.test.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index 3aa1c0a8e480..925e3fda1f98 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -1,30 +1,30 @@ -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + createContextFromParsedArgs, BreakingChangeLabelsToBeAdded, + getSwaggerDiffs, buildPrInfo, changeBaseBranch, - cleanDummySwagger, - createContextFromParsedArgs, + logFullOadMessagesList, createDummySwagger, - getCreatedDummySwaggerCount, - getSwaggerDiffs, + cleanDummySwagger, isSameVersionBreakingType, - logFullOadMessagesList, + getCreatedDummySwaggerCount, outputBreakingChangeLabelVariables, type ParsedCliArguments, } from "../src/command-helpers.js"; -import { LogLevel } from "../src/log.js"; import { - BreakingChangeReviewRequiredLabel, Context, + BreakingChangeReviewRequiredLabel, VersioningReviewRequiredLabel, } from "../src/types/breaking-change.js"; import { ResultMessageRecord } from "../src/types/message.js"; +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { LogLevel } from "../src/log.js"; // Test constants const TEST_CONSTANTS = { From 794ef8ef925c8ec8dcca737e3a9290e4ac26cbae Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:55:13 +0000 Subject: [PATCH 38/40] typo --- .../test/command-helpers.test.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index 925e3fda1f98..e562f3528608 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -1,30 +1,30 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { - createContextFromParsedArgs, BreakingChangeLabelsToBeAdded, - getSwaggerDiffs, buildPrInfo, changeBaseBranch, - logFullOadMessagesList, - createDummySwagger, cleanDummySwagger, - isSameVersionBreakingType, + createContextFromParsedArgs, + createDummySwagger, getCreatedDummySwaggerCount, + getSwaggerDiffs, + isSameVersionBreakingType, + logFullOadMessagesList, outputBreakingChangeLabelVariables, type ParsedCliArguments, } from "../src/command-helpers.js"; +import { LogLevel } from "../src/log.js"; import { - Context, BreakingChangeReviewRequiredLabel, + Context, VersioningReviewRequiredLabel, } from "../src/types/breaking-change.js"; import { ResultMessageRecord } from "../src/types/message.js"; -import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { LogLevel } from "../src/log.js"; // Test constants const TEST_CONSTANTS = { @@ -370,7 +370,7 @@ describe("command-helpers", () => { baseCommitish: TEST_CONSTANTS.BRANCHES.MAIN, cwd: TEST_CONSTANTS.PATHS.TEST_PATH, headCommitish: TEST_CONSTANTS.COMMITS.HEAD, - path: ["specification"], + paths: ["specification"], }); }); From ad136d9c9c3b53de7d91bf695deb150b21e812ec Mon Sep 17 00:00:00 2001 From: Mike Harder Date: Tue, 22 Jul 2025 21:55:59 +0000 Subject: [PATCH 39/40] revert imports --- .../test/command-helpers.test.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index e562f3528608..2aafa10fe5a1 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -1,30 +1,30 @@ -import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; -import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { + createContextFromParsedArgs, BreakingChangeLabelsToBeAdded, + getSwaggerDiffs, buildPrInfo, changeBaseBranch, - cleanDummySwagger, - createContextFromParsedArgs, + logFullOadMessagesList, createDummySwagger, - getCreatedDummySwaggerCount, - getSwaggerDiffs, + cleanDummySwagger, isSameVersionBreakingType, - logFullOadMessagesList, + getCreatedDummySwaggerCount, outputBreakingChangeLabelVariables, type ParsedCliArguments, } from "../src/command-helpers.js"; -import { LogLevel } from "../src/log.js"; import { - BreakingChangeReviewRequiredLabel, Context, + BreakingChangeReviewRequiredLabel, VersioningReviewRequiredLabel, } from "../src/types/breaking-change.js"; import { ResultMessageRecord } from "../src/types/message.js"; +import { getChangedFilesStatuses } from "@azure-tools/specs-shared/changed-files"; +import { BREAKING_CHANGES_CHECK_TYPES } from "@azure-tools/specs-shared/breaking-change"; +import { LogLevel } from "../src/log.js"; // Test constants const TEST_CONSTANTS = { From 55c4d3aa267112cdf0b1650dc2e694650f4b30a0 Mon Sep 17 00:00:00 2001 From: "Mike Harder (from Dev Box)" Date: Tue, 22 Jul 2025 18:24:51 -0700 Subject: [PATCH 40/40] [openapi-diff-runner] Remove unnecessary path mock --- eng/tools/openapi-diff-runner/test/command-helpers.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts index 2aafa10fe5a1..50a3d1228838 100644 --- a/eng/tools/openapi-diff-runner/test/command-helpers.test.ts +++ b/eng/tools/openapi-diff-runner/test/command-helpers.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { existsSync, mkdirSync, readFileSync, writeFileSync, rmSync } from "node:fs"; -import path from "node:path"; import { fileURLToPath } from "node:url"; import { createContextFromParsedArgs, @@ -186,7 +185,6 @@ function setupCommonMocks() { // Mock dependencies vi.mock("node:fs"); -vi.mock("node:path"); vi.mock("node:url"); vi.mock("../src/utils/common-utils.js"); vi.mock("../src/utils/oad-message-processor.js"); @@ -206,7 +204,6 @@ describe("command-helpers", () => { const mockReadFileSync = vi.mocked(readFileSync); const mockWriteFileSync = vi.mocked(writeFileSync); const mockRmSync = vi.mocked(rmSync); - const mockPath = vi.mocked(path); const mockFileURLToPath = vi.mocked(fileURLToPath); const mockGetChangedFilesStatuses = vi.mocked(getChangedFilesStatuses); @@ -214,9 +211,6 @@ describe("command-helpers", () => { vi.clearAllMocks(); // Setup default mocks - mockPath.resolve.mockImplementation((...paths) => paths.join("/")); - mockPath.join.mockImplementation((...paths) => paths.join("/")); - mockPath.dirname.mockImplementation((p) => p.split("/").slice(0, -1).join("/")); mockFileURLToPath.mockReturnValue("/path/to/file.js"); // Mock console methods to avoid test output noise