Fix: diff command (@W-19076728@) (#235)

* refactor diff

* fix updateApis command

* strip out minor and patch version from dir name

* remove console logs

* hide normalization under flag

* add unit tests

* update unit test

* remove unecessary test file

* update changelog

* update example

* invert logic to make normalization default

* update unit tests

Author: joeluong-sfcc

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## 0.8.0
+
+* Add `--disable-normalize-directory-names` flag to `diff` command. By default directory names will be normalized by major version, e.g: `shopper-stores-oas-1.0.16` --> `shopper-stores-oas-1` [#235](https://github.com/SalesforceCommerceCloud/raml-toolkit/pull/235/)
+
 ## 0.7.0
 
 * Add `ExchangeConfig` type [#230](https://github.com/SalesforceCommerceCloud/raml-toolkit/pull/230)

src/diff/diffCommand.ts

@@ -70,6 +70,11 @@ Exit statuses:
         "Specifies the API spec of the files being compared. Defaults to RAML. Options are RAML or OAS",
       options: ["raml", "oas"],
     }),
+    "disable-normalize-directory-names": flags.boolean({
+      description:
+        "Disable normalization of directory names. When enabled, directory names will keep their full version numbers (e.g., 'shopper-stores-oas-1.0.16' instead of 'shopper-stores-oas-1')",
+      default: false,
+    }),
   };
 
   static args = [

src/diff/oasDiff.test.ts

@@ -9,6 +9,7 @@
 import { expect } from "chai";
 import proxyquire from "proxyquire";
 import sinon from "sinon";
+import { consoleStub } from "../../testResources/setup";
 
 const pq = proxyquire.noCallThru();
 
@@ -678,4 +679,84 @@ describe("oasDiffChangelog", () => {
     expect(execStub.called).to.be.true;
     expect(result).to.equal(1); // Changes should be reported
   });
+
+  it("should normalize directory names when disable-normalize-directory-names flag is false", async () => {
+    const execStub = createMockExec();
+    execStub.onSecondCall().callsArgWith(1, null, "changes in api-1", "");
+
+    const fsStub = createMockFs();
+    // Setup directory structure with versioned directory names
+    setupDirectoryStructure(fsStub, [
+      { path: "base", contents: ["api-1.0.16", "api-2.1.5"] },
+      { path: "base/api-1.0.16", contents: ["exchange.json", "spec.yaml"] },
+      { path: "base/api-2.1.5", contents: ["exchange.json", "spec.yaml"] },
+      { path: "new", contents: ["api-1.0.17", "api-2.1.6"] },
+      { path: "new/api-1.0.17", contents: ["exchange.json", "spec.yaml"] },
+      { path: "new/api-2.1.6", contents: ["exchange.json", "spec.yaml"] },
+    ]);
+
+    const oasDiff = createOasDiffProxy(execStub, fsStub);
+
+    const baseApi = "base";
+    const newApi = "new";
+    const flags = {
+      "out-file": "output.txt",
+      dir: true,
+      "disable-normalize-directory-names": false,
+    };
+
+    await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
+
+    expect(fsStub.writeFile.called).to.be.true;
+    const writtenContent = fsStub.writeFile.args[0][1];
+
+    // Verify that the function completes successfully with the normalize-directory-names flag enabled
+    expect(writtenContent).to.be.a("string");
+    expect(writtenContent.length).to.be.greaterThan(0);
+
+    // Verify that the console.log stub captured the "Processing directory pair" messages
+    expect(consoleStub.called).to.be.true;
+    // flattens the array of arrays into a single array of strings
+    const allMessages = consoleStub.args.map((args) => args[0]);
+    expect(allMessages).to.include("Processing directory pair: api-1");
+    expect(allMessages).to.include("Processing directory pair: api-2");
+    consoleStub.restore();
+  });
+
+  it("should handle fs.readdir error in findYamlFiles function", async () => {
+    const consoleWarnSpy = sinon.spy(console, "warn");
+    const execStub = createMockExec();
+    execStub.onSecondCall().callsArgWith(1, null, "changes in api-v1", "");
+
+    const fsStub = createMockFs();
+    setupDirectoryStructure(fsStub, [
+      { path: "base", contents: ["api-v1"] },
+      { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] },
+      { path: "new", contents: ["api-v1"] },
+      { path: "new/api-v1", contents: ["exchange.json", "spec.yaml"] },
+    ]);
+
+    // Make readdir fail for the new/api-v1 directory to trigger the error handling branch
+    fsStub.readdir
+      .withArgs("new/api-v1")
+      .rejects(new Error("Permission denied"));
+
+    const oasDiff = createOasDiffProxy(execStub, fsStub);
+
+    const baseApi = "base";
+    const newApi = "new";
+    const flags = {
+      dir: true,
+    };
+
+    await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
+
+    // Verify that the function handles the error gracefully
+    expect(consoleWarnSpy.called).to.be.true;
+    expect(consoleWarnSpy.args[0][0]).to.include(
+      "Warning: Could not read directory new/api-v1:"
+    );
+    expect(consoleWarnSpy.args[0][1]).to.include("Permission denied");
+    consoleWarnSpy.restore();
+  });
 });

src/diff/oasDiff.ts

@@ -17,6 +17,7 @@ interface OasDiffFlags {
   format?: "json" | "console";
   dir?: boolean;
   "out-file"?: string;
+  "disable-normalize-directory-names"?: boolean;
 }
 
 /**
@@ -242,6 +243,28 @@ async function executeOasDiff(
   });
 }
 
+/**
+ * Remove minor and patch versions from directory names and keep only major version
+ * Example: 'shopper-stores-oas-1.0.16' -> 'shopper-stores-oas-1'
+ * We want to keep the major version for multiple versions of the same API, ie: Shopper Baskets
+ *
+ * @param dirs - Array of directory objects with name and path properties
+ * @returns Copy of passed array with updated name property
+ */
+function normalizeDirectoryNames(
+  dirs: Array<{ name: string; path: string }>
+): Array<{ name: string; path: string }> {
+  return dirs.map((dir) => ({
+    ...dir,
+    // matches the pattern of the version number in the directory name, ie: -1.0.16
+    // extracts the major version number if available, otherwise the original match if no major version is found
+    name: dir.name.replace(/-\d+\.\d+\.\d+$/, (match) => {
+      const majorVersion = match.match(/^-\d+/)?.[0];
+      return majorVersion || match;
+    }),
+  }));
+}
+
 /**
  * Handle directory mode comparison logic
  *
@@ -262,8 +285,13 @@ async function handleDirectoryMode(
   let hasErrors = false;
 
   // Find all exchange.json files and their parent directories
-  const baseExchangeDirs = await findExchangeDirectories(baseApi);
-  const newExchangeDirs = await findExchangeDirectories(newApi);
+  let baseExchangeDirs = await findExchangeDirectories(baseApi);
+  let newExchangeDirs = await findExchangeDirectories(newApi);
+
+  if (!flags["disable-normalize-directory-names"]) {
+    baseExchangeDirs = normalizeDirectoryNames(baseExchangeDirs);
+    newExchangeDirs = normalizeDirectoryNames(newExchangeDirs);
+  }
 
   const allDirNames = new Set([
     ...baseExchangeDirs.map((dir) => dir.name),

src/download/exchangeDownloader.ts

@@ -434,6 +434,7 @@ function getLatestReleaseVersion(versionGroup: {
   const releaseAssetVersions = versionGroup.versions.filter((version) => {
     return releaseSemverRegex.test(version.version);
   });
+
   // Sort versions and get the latest
   return releaseAssetVersions.sort((instanceA, instanceB) => {
     const [aMajor, aMinor, aPatch] = instanceA.version.split(".").map(Number);
@@ -442,5 +443,5 @@ function getLatestReleaseVersion(versionGroup: {
     if (aMajor !== bMajor) return bMajor - aMajor;
     if (aMinor !== bMinor) return bMinor - aMinor;
     return bPatch - aPatch;
-  })[0].version;
+  })[0]?.version;
 }

testResources/setup.ts

@@ -7,8 +7,8 @@
 import sinon from "sinon";
 import { ramlToolLogger } from "../src/common/logger";
 
-let ramlToolLoggerStub: sinon.SinonStub;
-let consoleStub: sinon.SinonStub;
+export let ramlToolLoggerStub: sinon.SinonStub;
+export let consoleStub: sinon.SinonStub;
 
 beforeEach(() => {
   ramlToolLoggerStub = sinon.stub(ramlToolLogger, "info");