diff --git a/eng/tools/lint-diff/src/correlateResults.ts b/eng/tools/lint-diff/src/correlateResults.ts index 9e7cb660bb75..f69ed1eb5ece 100644 --- a/eng/tools/lint-diff/src/correlateResults.ts +++ b/eng/tools/lint-diff/src/correlateResults.ts @@ -43,9 +43,7 @@ export async function correlateRuns( throw new Error(`No default tag found for readme ${readme} in before state`); } const beforeDefaultTagCandidates = beforeChecks.filter( - (r) => - relative(beforePath, r.readme.path) === readmePathRelative && - (r.tag === defaultTag || r.tag == ""), + (r) => relative(beforePath, r.readme.path) === readmePathRelative && r.tag === defaultTag, ); if (beforeDefaultTagCandidates.length === 1) { diff --git a/eng/tools/lint-diff/src/processChanges.ts b/eng/tools/lint-diff/src/processChanges.ts index 3efb617a1135..ea2400474d46 100644 --- a/eng/tools/lint-diff/src/processChanges.ts +++ b/eng/tools/lint-diff/src/processChanges.ts @@ -148,7 +148,7 @@ export async function buildState( if (!changedFileAndTagsMap.has(changedReadme)) { changedFileAndTagsMap.set(changedReadme, { readme: readmeObject, - changedTags: new Set([""]), + changedTags: new Set(), }); } } @@ -205,22 +205,6 @@ export function reconcileChangedFilesAndTags( }); } - // If a tag exists in after but not in before, make sure that the the default - // tag from before is included in scans - for (const [readme, afterTags] of afterFinal.entries()) { - if (!beforeFinal.has(readme)) { - continue; - } - - const beforeTags = beforeFinal.get(readme)!.changedTags; - for (const tag of afterTags.changedTags) { - if (!beforeTags.has(tag)) { - beforeTags.add(""); - break; - } - } - } - return [beforeFinal, afterFinal]; } diff --git a/eng/tools/lint-diff/src/runChecks.ts b/eng/tools/lint-diff/src/runChecks.ts index d697decedfaa..ec7cc6807197 100644 --- a/eng/tools/lint-diff/src/runChecks.ts +++ b/eng/tools/lint-diff/src/runChecks.ts @@ -31,11 +31,9 @@ export async function runChecks( // and overriding openapi-type with it. let openApiSubType = openApiType; - if (tags.changedTags.size === 0) { - throw new Error(`No changed tags found for readme ${readme}`); - } - - for (const tag of tags.changedTags) { + // If the tags array is empty run the loop once but with a null tag + const coalescedTags = tags.changedTags?.size ? [...tags.changedTags] : [null]; + for (const tag of coalescedTags) { console.log(`::group::Autorest for type: ${openApiType} readme: ${readme} tag: ${tag}`); const autorestArgs = [ @@ -69,7 +67,7 @@ export async function runChecks( autorestCommand, rootPath: path, readme: tags.readme, - tag: tag, + tag: tag ? tag : "", openApiType, error: null, ...executionResult, @@ -84,7 +82,7 @@ export async function runChecks( autorestCommand, rootPath: path, readme: tags.readme, - tag: tag, + tag: tag ? tag : "", openApiType, error: execError, stdout: execError.stdout || "", diff --git a/eng/tools/lint-diff/test/correlateResults.test.ts b/eng/tools/lint-diff/test/correlateResults.test.ts index e4a96f5c84c4..7f06982fe783 100644 --- a/eng/tools/lint-diff/test/correlateResults.test.ts +++ b/eng/tools/lint-diff/test/correlateResults.test.ts @@ -120,92 +120,6 @@ describe.skipIf(isWindows())("correlateRuns", () => { }); }); - test("correlates before and after runs with matching readme and empty string tag", async () => { - const fixtureRoot = resolve(__dirname, "fixtures/correlateRuns"); - const beforePath = resolve(fixtureRoot, "before"); - const afterPath = resolve(fixtureRoot, "after"); - - const beforeChecks: AutorestRunResult[] = [ - { - rootPath: beforePath, - readme: new Readme( - resolve(beforePath, "specification/service1/resource-manager/readme.md"), - ), - tag: "", - stdout: "stdout", - stderr: "stderr", - error: null, - }, - ]; - - const afterChecks: AutorestRunResult[] = [ - { - rootPath: afterPath, - readme: new Readme(resolve(afterPath, "specification/service1/resource-manager/readme.md")), - tag: "tag2", - stdout: "stdout", - stderr: "stderr", - error: null, - }, - ]; - - const result = await correlateRuns(beforePath, beforeChecks, afterChecks); - expect(result.size).toEqual(1); - expect(result.get("specification/service1/resource-manager/readme.md#tag2")).toMatchObject({ - before: beforeChecks[0], - after: afterChecks[0], - }); - }); - - test("correlates before and multiple after runs with matching readme and empty string tag", async () => { - const fixtureRoot = resolve(__dirname, "fixtures/correlateRuns"); - const beforePath = resolve(fixtureRoot, "before"); - const afterPath = resolve(fixtureRoot, "after"); - - const beforeChecks: AutorestRunResult[] = [ - { - rootPath: beforePath, - readme: new Readme( - resolve(beforePath, "specification/service1/resource-manager/readme.md"), - ), - tag: "", - stdout: "stdout", - stderr: "stderr", - error: null, - }, - ]; - - const afterChecks: AutorestRunResult[] = [ - { - rootPath: afterPath, - readme: new Readme(resolve(afterPath, "specification/service1/resource-manager/readme.md")), - tag: "tag2", - stdout: "stdout", - stderr: "stderr", - error: null, - }, - { - rootPath: afterPath, - readme: new Readme(resolve(afterPath, "specification/service1/resource-manager/readme.md")), - tag: "tag3", - stdout: "stdout", - stderr: "stderr", - error: null, - }, - ]; - - const result = await correlateRuns(beforePath, beforeChecks, afterChecks); - expect(result.size).toEqual(2); - expect(result.get("specification/service1/resource-manager/readme.md#tag2")).toMatchObject({ - before: beforeChecks[0], - after: afterChecks[0], - }); - expect(result.get("specification/service1/resource-manager/readme.md#tag3")).toMatchObject({ - before: beforeChecks[0], - after: afterChecks[1], - }); - }); - test("uses no baseline if there are no matching before checks", async () => { const fixtureRoot = resolve(__dirname, "fixtures/correlateRuns"); const beforePath = resolve(fixtureRoot, "before"); diff --git a/eng/tools/lint-diff/test/processChanges.test.ts b/eng/tools/lint-diff/test/processChanges.test.ts index 0f0d933c31ca..7224e4978994 100644 --- a/eng/tools/lint-diff/test/processChanges.test.ts +++ b/eng/tools/lint-diff/test/processChanges.test.ts @@ -82,13 +82,12 @@ describe("reconcileChangedFilesAndTags", () => { const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after); expect(beforeFinal).toEqual( - new Map([ + new Map([ [ "specification/1/readme.md", - { - readme: expect.any(Readme), + expect.objectContaining({ changedTags: new Set(["tag1"]), - }, + }), ], ]), ); @@ -136,96 +135,6 @@ describe("reconcileChangedFilesAndTags", () => { expect(beforeFinal).toEqual(before); expect(afterFinal).toEqual(after); }); - - test("adds an empty tag to beforeFinal if after has a new tag", () => { - const before = new Map([ - [ - "specification/1/readme.md", - { - readme: new Readme("specification/1/readme.md"), - changedTags: new Set(["tag2"]), - }, - ], - ]); - const after = new Map([ - [ - "specification/1/readme.md", - { - readme: new Readme("specification/1/readme.md"), - changedTags: new Set(["tag2", "tag3"]), - }, - ], - ]); - - const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after); - - expect(beforeFinal).toEqual( - new Map([ - [ - "specification/1/readme.md", - { - readme: expect.any(Readme), - changedTags: new Set(["tag2", ""]), - }, - ], - ]), - ); - expect(afterFinal).toEqual(after); - }); - - test("adds an empty tag to beforeFinal if after has multiple new tags", () => { - const before = new Map([ - [ - "specification/1/readme.md", - { - readme: new Readme("specification/1/readme.md"), - changedTags: new Set(["tag2"]), - }, - ], - ]); - const after = new Map([ - [ - "specification/1/readme.md", - { - readme: new Readme("specification/1/readme.md"), - changedTags: new Set(["tag2", "tag3", "tag4"]), - }, - ], - ]); - - const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after); - - expect(beforeFinal).toEqual( - new Map([ - [ - "specification/1/readme.md", - { - readme: expect.any(Readme), - changedTags: new Set(["tag2", ""]), - }, - ], - ]), - ); - expect(afterFinal).toEqual(after); - }); - - test("handles a new specification in after", () => { - const before = new Map(); - const after = new Map([ - [ - "specification/1/readme.md", - { - readme: new Readme("specification/1/readme.md"), - changedTags: new Set(["tag1"]), - }, - ], - ]); - - const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after); - - expect(beforeFinal).toEqual(new Map()); - expect(afterFinal).toEqual(after); - }); }); describe("getChangedSwaggers", () => { @@ -319,7 +228,7 @@ describe("buildState", () => { [ "specification/edit-in-place/readme.md", { - changedTags: new Set([""]), + changedTags: new Set(), readme: expect.any(Readme), }, ], @@ -333,7 +242,7 @@ describe("buildState", () => { }); test("does not throw if a file is missing", async () => { - await expect(() => + expect(() => buildState( ["specification/edit-in-place/data-plane/does-not-exist.json"], "test/fixtures/buildState/", diff --git a/eng/tools/lint-diff/test/runChecks.test.ts b/eng/tools/lint-diff/test/runChecks.test.ts index f0f43321d4e6..5dd16b881dfc 100644 --- a/eng/tools/lint-diff/test/runChecks.test.ts +++ b/eng/tools/lint-diff/test/runChecks.test.ts @@ -52,14 +52,18 @@ describe("runChecks", () => { ); }); - test("throws when no tags specified", async () => { + test("coalesces null tag when no tags specified", async () => { (execNpmExec as Mock).mockResolvedValue({ stdout: "", stderr: "" }); const runList = new Map([ ["readme.md", { readme: new Readme(""), changedTags: new Set() }], ]); - const actual = runChecks("root", runList); - await expect(actual).rejects.toThrowError("No changed tags found for readme"); + const actual = await runChecks("root", runList); + expect(actual).toHaveLength(1); + expect(execNpmExec).toHaveBeenCalledWith( + expect.not.arrayContaining([expect.stringContaining("--tag")]), + expect.anything(), + ); }); test("error path populates error, stdout, stderr", async () => {