Skip to content

Revert "LintDiff: Ensure that default tag is scanned when there is a new tag in the 'after' state" #38660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danieljurek merged 1 commit into from
Nov 7, 2025
Merged

Revert "LintDiff: Ensure that default tag is scanned when there is a new tag in the 'after' state" #38660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions eng/tools/lint-diff/src/correlateResults.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { Readme } from "@azure-tools/specs-shared/readme";
import { basename, join, relative } from "path";
import { AutorestRunResult, BeforeAfter, LintDiffViolation, Source } from "./lintdiff-types.js";
Expand Down Expand Up @@ -43,9 +43,7 @@
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) {
Expand Down
18 changes: 1 addition & 17 deletions eng/tools/lint-diff/src/processChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { readme, swagger } from "@azure-tools/specs-shared/changed-files";
import { SpecModel } from "@azure-tools/specs-shared/spec-model";
import deepEqual from "deep-eql";
Expand Down Expand Up @@ -148,7 +148,7 @@
if (!changedFileAndTagsMap.has(changedReadme)) {
changedFileAndTagsMap.set(changedReadme, {
readme: readmeObject,
changedTags: new Set<string>([""]),
changedTags: new Set<string>(),
});
}
}
Expand Down Expand Up @@ -205,22 +205,6 @@
});
}

// 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];
}

Expand Down
12 changes: 5 additions & 7 deletions eng/tools/lint-diff/src/runChecks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { ExecError, execNpmExec, isExecError } from "@azure-tools/specs-shared/exec";
import { debugLogger } from "@azure-tools/specs-shared/logger";
import { join } from "path";
Expand Down Expand Up @@ -31,11 +31,9 @@
// 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 = [
Expand Down Expand Up @@ -69,7 +67,7 @@
autorestCommand,
rootPath: path,
readme: tags.readme,
tag: tag,
tag: tag ? tag : "",
openApiType,
error: null,
...executionResult,
Expand All @@ -84,7 +82,7 @@
autorestCommand,
rootPath: path,
readme: tags.readme,
tag: tag,
tag: tag ? tag : "",
openApiType,
error: execError,
stdout: execError.stdout || "",
Expand Down
86 changes: 0 additions & 86 deletions eng/tools/lint-diff/test/correlateResults.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { describe, expect, test } from "vitest";

import { Readme } from "@azure-tools/specs-shared/readme";
Expand Down Expand Up @@ -120,92 +120,6 @@
});
});

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");
Expand Down
101 changes: 5 additions & 96 deletions eng/tools/lint-diff/test/processChanges.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { describe, expect, test } from "vitest";

import { ReadmeAffectedTags } from "../src/lintdiff-types.js";
Expand Down Expand Up @@ -82,13 +82,12 @@

const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after);
expect(beforeFinal).toEqual(
new Map<string, ReadmeAffectedTags>([
new Map<string, string[]>([
[
"specification/1/readme.md",
{
readme: expect.any(Readme),
expect.objectContaining({
changedTags: new Set<string>(["tag1"]),
},
}),
],
]),
);
Expand Down Expand Up @@ -136,96 +135,6 @@
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<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: new Readme("specification/1/readme.md"),
changedTags: new Set<string>(["tag2"]),
},
],
]);
const after = new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: new Readme("specification/1/readme.md"),
changedTags: new Set<string>(["tag2", "tag3"]),
},
],
]);

const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after);

expect(beforeFinal).toEqual(
new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: expect.any(Readme),
changedTags: new Set<string>(["tag2", ""]),
},
],
]),
);
expect(afterFinal).toEqual(after);
});

test("adds an empty tag to beforeFinal if after has multiple new tags", () => {
const before = new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: new Readme("specification/1/readme.md"),
changedTags: new Set<string>(["tag2"]),
},
],
]);
const after = new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: new Readme("specification/1/readme.md"),
changedTags: new Set<string>(["tag2", "tag3", "tag4"]),
},
],
]);

const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after);

expect(beforeFinal).toEqual(
new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: expect.any(Readme),
changedTags: new Set<string>(["tag2", ""]),
},
],
]),
);
expect(afterFinal).toEqual(after);
});

test("handles a new specification in after", () => {
const before = new Map<string, ReadmeAffectedTags>();
const after = new Map<string, ReadmeAffectedTags>([
[
"specification/1/readme.md",
{
readme: new Readme("specification/1/readme.md"),
changedTags: new Set<string>(["tag1"]),
},
],
]);

const [beforeFinal, afterFinal] = reconcileChangedFilesAndTags(before, after);

expect(beforeFinal).toEqual(new Map<string, ReadmeAffectedTags>());
expect(afterFinal).toEqual(after);
});
});

describe("getChangedSwaggers", () => {
Expand Down Expand Up @@ -319,7 +228,7 @@
[
"specification/edit-in-place/readme.md",
{
changedTags: new Set<string>([""]),
changedTags: new Set<string>(),
readme: expect.any(Readme),
},
],
Expand All @@ -333,7 +242,7 @@
});

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/",
Expand Down
10 changes: 7 additions & 3 deletions eng/tools/lint-diff/test/runChecks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import { beforeEach, describe, expect, Mock, test, vi } from "vitest";

vi.mock(import("@azure-tools/specs-shared/exec"), async (importOriginal) => {
Expand Down Expand Up @@ -52,14 +52,18 @@
);
});

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<string, ReadmeAffectedTags>([
["readme.md", { readme: new Readme(""), changedTags: new Set<string>() }],
]);

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 () => {
Expand Down
Loading