[openapi-diff-runner] merge updates from main

eng/tools/openapi-diff-runner/src/detect-breaking-change.ts

@@ -41,7 +41,11 @@ import {
   specIsPreview,
 } from "./utils/common-utils.js";
 import { processAndAppendOadMessages } from "./utils/oad-message-processor.js";
-import { getExistedVersionOperations, getPrecedingSwaggers } from "./utils/spec.js";
+import {
+  deduplicateSwaggers,
+  getExistedVersionOperations,
+  getPrecedingSwaggers,
+} from "./utils/spec.js";
 
 // We want to display some lines as we improved AutoRest v2 error output since March 2024 to provide multi-line error messages, e.g.:
 // https://github.com/Azure/autorest/pull/4934
@@ -182,11 +186,14 @@ export async function checkCrossVersionBreakingChange(
       continue;
     }
 
-    const availableSwaggers = await specModel.getSwaggers();
+    const originalReturnedSwaggers = await specModel.getSwaggers();
+    const availableSwaggers = deduplicateSwaggers(originalReturnedSwaggers);
     logMessage(
       `checkCrossVersionBreakingChange: swaggerPath: ${swaggerPath}, availableSwaggers.length: ${availableSwaggers?.length}`,
     );
-    const previousVersions = await getPrecedingSwaggers(swaggerPath, availableSwaggers);
+
+    // use absoluteSwaggerPath as it will need to it will fall back to use the version info in the swagger content
+    const previousVersions = await getPrecedingSwaggers(absoluteSwaggerPath, availableSwaggers);
     logMessage(
       `checkCrossVersionBreakingChange: previousVersions: ${JSON.stringify(previousVersions)}`,
     );
@@ -355,35 +362,49 @@ export function isInDevFolder(swaggerPath: string) {
 }
 
 /**
+ * Find the path to the closest readme.md file based on the input file path,
+ * searching upward from the file's directory up to the 'resource-manager' or 'data-plane' sub path.
  * Example:
  * input: specification/network/resource-manager/Microsoft.Network/stable/2019-11-01/network.json
- * returns: specification/network/resource-manager
+ * returns: path to the directory containing the closest readme.md file, up to specification/network/resource-manager
  */
 export function getReadmeFolder(swaggerFile: string) {
   const segments = swaggerFile.split(/\\|\//);
 
-  if (segments && segments.length >= 3) {
-    // Handle dev folder conversion
-    if (segments[0] === "dev") {
-      segments[0] = "specification";
-    }
+  if (!segments || segments.length < 3) {
+    return undefined;
+  }
 
-    // Look for "resource-manager" or "data-plane" in the path
-    const resourceManagerIndex = segments.findIndex((segment) => segment === "resource-manager");
-    if (resourceManagerIndex !== -1) {
-      return segments.slice(0, resourceManagerIndex + 1).join("/");
-    }
+  // Handle dev folder conversion
+  if (segments[0] === "dev") {
+    segments[0] = "specification";
+  }
+
+  // Find the boundary index (resource-manager or data-plane)
+  const resourceManagerIndex = segments.findIndex((segment) => segment === "resource-manager");
+  const dataPlaneIndex = segments.findIndex((segment) => segment === "data-plane");
+  const boundaryIndex = resourceManagerIndex !== -1 ? resourceManagerIndex : dataPlaneIndex;
+
+  // Determine search range: from file's parent directory up to boundary (or root if no boundary found)
+  const startIndex = segments.length - 2; // Start from parent directory of the file
+  const minIndex = boundaryIndex !== -1 ? boundaryIndex : 2; // Stop at boundary or at least keep first 3 segments
 
-    const dataPlaneIndex = segments.findIndex((segment) => segment === "data-plane");
-    if (dataPlaneIndex !== -1) {
-      return segments.slice(0, dataPlaneIndex + 1).join("/");
+  // Search upward for readme.md
+  for (let i = startIndex; i >= minIndex; i--) {
+    const currentPath = segments.slice(0, i + 1).join(path.sep);
+    const readmePath = path.join(currentPath, "readme.md");
+
+    if (existsSync(readmePath)) {
+      return currentPath;
     }
+  }
 
-    // Default: return first 3 segments
-    return segments.slice(0, 3).join("/");
+  // Fallback: return up to boundary if found, otherwise first 3 segments
+  if (boundaryIndex !== -1) {
+    return segments.slice(0, boundaryIndex + 1).join(path.sep);
   }
 
-  return undefined;
+  return segments.slice(0, 3).join(path.sep);
 }
 
 /**
@@ -400,10 +421,37 @@ export function getSpecModel(specRepoFolder: string, swaggerPath: string): SpecM
     throw new Error(`Could not determine readme folder for swagger path: ${swaggerPath}`);
   }
 
-  const fullFolderPath = path.join(specRepoFolder, folder);
+  let isNewRp = false;
+  let fullFolderPath = path.join(specRepoFolder, folder);
+  if (!existsSync(fullFolderPath)) {
+    isNewRp = true;
+  }
+
+  // If the initial folder doesn't exist, search upward for a folder with readme.md
+  while (
+    isNewRp &&
+    !fullFolderPath.endsWith("resource-manager") &&
+    !fullFolderPath.endsWith("data-plane")
+  ) {
+    const parent = path.dirname(fullFolderPath);
+
+    // Prevent infinite loop if we reach the root
+    if (parent === fullFolderPath) {
+      break;
+    }
+
+    fullFolderPath = parent;
+    const readmePath = path.join(fullFolderPath, "readme.md");
+
+    // If we find a readme.md, use this folder
+    if (existsSync(readmePath)) {
+      isNewRp = false;
+      break;
+    }
+  }
 
   // Return if the readme folder is not found which means it's a new RP
-  if (!existsSync(fullFolderPath)) {
+  if (isNewRp) {
     logMessage(
       `getSpecModel: this is a new RP as ${fullFolderPath} folder does not exist in the base branch of spec repo.`,
     );
@@ -437,7 +485,9 @@ export async function checkAPIsBeingMovedToANewSpec(
     return;
   }
   const targetOperations = await targetSwagger.getOperations();
-  const movedApis = await getExistedVersionOperations(swaggerPath, availableSwaggers, [
+
+  // use absoluteSwaggerPath as it will need to it will fall back to use the version info in the swagger content
+  const movedApis = await getExistedVersionOperations(absoluteSwaggerPath, availableSwaggers, [
     ...targetOperations.values(),
   ]);
   logMessage(

eng/tools/openapi-diff-runner/src/utils/common-utils.ts

@@ -1,3 +1,4 @@
+import { existsSync, readFileSync } from "node:fs";
 import { Context } from "../types/breaking-change.js";
 import { FilePosition } from "../types/message.js";
 
@@ -111,13 +112,11 @@ export function getVersionFromInputFile(filePath: string, withPreview = false):
     }
   }
 
-  // If no regex match found, return the immediate folder name (parent directory of the file)
-  if (segments && segments.length > 1) {
-    // Get the folder name that contains the file (last segment before filename)
-    const folderName = segments[segments.length - 2];
-    if (folderName) {
-      return folderName;
-    }
+  // If no regex match found, try to read version from file content
+  if (existsSync(filePath)) {
+    const fileContent = readFileSync(filePath, "utf8");
+    const parsedContent = JSON.parse(fileContent);
+    return parsedContent?.info?.version || "";
   }
 
   return "";

eng/tools/openapi-diff-runner/test/utils/common-utils.test.ts

@@ -1,3 +1,4 @@
+import { existsSync, readFileSync } from "node:fs";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { Context } from "../../src/types/breaking-change.js";
 import {
@@ -17,6 +18,16 @@ import {
   targetHref,
 } from "../../src/utils/common-utils.js";
 
+// Mock node:fs module for file content parsing tests
+vi.mock("node:fs", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:fs")>();
+  return {
+    ...actual,
+    existsSync: vi.fn(),
+    readFileSync: vi.fn(),
+  };
+});
+
 describe("common-utils", () => {
   let originalEnv: NodeJS.ProcessEnv;
 
@@ -225,6 +236,11 @@ describe("common-utils", () => {
   });
 
   describe("getVersionFromInputFile", () => {
+    beforeEach(() => {
+      vi.mocked(existsSync).mockReset();
+      vi.mocked(readFileSync).mockReset();
+    });
+
     it("should extract version from data-plane path", () => {
       const result = getVersionFromInputFile(TEST_CONSTANTS.DATA_PLANE_PATH);
       expect(result).toBe(TEST_CONSTANTS.VERSION);
@@ -245,9 +261,96 @@ describe("common-utils", () => {
       expect(result).toBe("");
     });
 
-    it("should return folder name when no valid API version found", () => {
+    it("should return empty string when no valid API version found", () => {
       const result = getVersionFromInputFile("invalid/path.json");
-      expect(result).toBe("invalid");
+      expect(result).toBe("");
+    });
+
+    it("should extract version from file content when path regex fails and file exists", () => {
+      const filePath = "some/custom/path/spec.json";
+      const mockFileContent = JSON.stringify({
+        info: {
+          version: "2023-05-01",
+        },
+      });
+
+      vi.mocked(existsSync).mockReturnValue(true);
+      vi.mocked(readFileSync).mockReturnValue(mockFileContent);
+
+      const result = getVersionFromInputFile(filePath);
+      expect(result).toBe("2023-05-01");
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).toHaveBeenCalledWith(filePath, "utf8");
+    });
+
+    it("should extract preview version from file content when includePreview is true", () => {
+      const filePath = "some/custom/path/spec.json";
+      const mockFileContent = JSON.stringify({
+        info: {
+          version: "2023-05-01-preview",
+        },
+      });
+
+      vi.mocked(existsSync).mockReturnValue(true);
+      vi.mocked(readFileSync).mockReturnValue(mockFileContent);
+
+      const result = getVersionFromInputFile(filePath, true);
+      expect(result).toBe("2023-05-01-preview");
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).toHaveBeenCalledWith(filePath, "utf8");
+    });
+
+    it("should return empty string when file content has no version", () => {
+      const filePath = "some/custom/path/spec.json";
+      const mockFileContent = JSON.stringify({
+        info: {
+          title: "Test API",
+        },
+      });
+
+      vi.mocked(existsSync).mockReturnValue(true);
+      vi.mocked(readFileSync).mockReturnValue(mockFileContent);
+
+      const result = getVersionFromInputFile(filePath);
+      expect(result).toBe("");
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).toHaveBeenCalledWith(filePath, "utf8");
+    });
+
+    it("should throw error when file content is invalid JSON", () => {
+      const filePath = "some/custom/path/spec.json";
+      const mockFileContent = "invalid json content";
+
+      vi.mocked(existsSync).mockReturnValue(true);
+      vi.mocked(readFileSync).mockReturnValue(mockFileContent);
+
+      expect(() => getVersionFromInputFile(filePath)).toThrow();
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).toHaveBeenCalledWith(filePath, "utf8");
+    });
+
+    it("should return empty string when file does not exist and path regex fails", () => {
+      const filePath = "non/existent/path/spec.json";
+
+      vi.mocked(existsSync).mockReturnValue(false);
+
+      const result = getVersionFromInputFile(filePath);
+      expect(result).toBe("");
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).not.toHaveBeenCalled();
+    });
+
+    it("should throw error when file read fails", () => {
+      const filePath = "some/custom/path/spec.json";
+
+      vi.mocked(existsSync).mockReturnValue(true);
+      vi.mocked(readFileSync).mockImplementation(() => {
+        throw new Error("File read error");
+      });
+
+      expect(() => getVersionFromInputFile(filePath)).toThrow("File read error");
+      expect(vi.mocked(existsSync)).toHaveBeenCalledWith(filePath);
+      expect(vi.mocked(readFileSync)).toHaveBeenCalledWith(filePath, "utf8");
     });
   });