Refactor to support diffCommand test

vcua-mobify · vcua-mobify · commit 7f32c78e836b · 2025-05-16T16:40:33.000-07:00
diff --git a/src/diff/diffCommand.test.ts b/src/diff/diffCommand.test.ts
@@ -7,22 +7,27 @@
 "use strict";
 
 import { expect, test } from "@oclif/test";
-import { writeSync } from "fs-extra";
+import * as fs from "fs-extra";
 import { FileResult, fileSync, dirSync, DirResult } from "tmp";
 import { Rule } from "json-rules-engine";
 import chai from "chai";
 import chaiFs from "chai-fs";
 
-import { DiffCommand as cmd } from "./diffCommand";
+import { DiffCommand as cmd, DiffCommand } from "./diffCommand";
 import * as diffDirectories from "./diffDirectories";
 import { NodeChanges } from "./changes/nodeChanges";
 import { ApiChanges } from "./changes/apiChanges";
 import { ApiCollectionChanges } from "./changes/apiCollectionChanges";
 import { ApiDifferencer } from "./apiDifferencer";
 import { CategorizedChange } from "./changes/categorizedChange";
 import { RuleCategory } from "./ruleCategory";
+import * as oasDiff from "./oasDiff";
+
+import proxyquire from "proxyquire";
+import sinon from "sinon";
 
 chai.use(chaiFs);
+const pq = proxyquire.noCallThru();
 
 const nodeChanges = new NodeChanges("test-id", ["test:type"]);
 nodeChanges.added = { "core:name": "oldName" };
@@ -40,7 +45,7 @@ apiCollectionChanges.changed["file.raml"] = apiChanges;
 
 const createTempFile = (content: string): FileResult => {
   const tempFile = fileSync();
-  writeSync(tempFile.fd, content);
+  fs.writeSync(tempFile.fd, content);
   return tempFile;
 };
 
@@ -71,6 +76,43 @@ const ramlAdded = createTempFile(
 `
 );
 
+const oasAdded = createTempFile(
+  `
+    "openapi": "3.0.0",
+    "info": {
+      "title": "Test API",
+      "version": "1.0.0"
+    },
+    "paths": {
+      "/resource": {
+        "get": {
+          "summary": "Get resource"
+        },
+        "post": {
+          "summary": "Create resource"
+        }
+      }
+    }
+  `
+);
+
+const oasOld = createTempFile(
+  `
+    "openapi": "3.0.0",
+    "info": {
+      "title": "Test API",
+      "version": "1.0.0"
+    },
+    "paths": {
+      "/resource": {
+        "get": {
+          "summary": "Get resource"
+        }
+      }
+    }
+  `
+);
+
 const operationRemovedRule = new Rule(
   `{
     "name": "Rule to detect operation removal",
@@ -376,4 +418,15 @@ describe("raml-toolkit cli diff command", () => {
     )
     .exit(2)
     .it("does not allow format to be specified for diff only");
+
+  // TODO: The stub does not apply. The actual oasDiffChangelog is called and it will throw
+  // an error because we are using test data and not actual oas files
+  test
+    .stderr({ print: true })
+    .stdout({ print: true })
+    .stub(oasDiff, "oasDiffChangelog", async () => 0)
+    .do(() => {
+      cmd.run([oasOld.name, oasAdded.name, "-s", "oas"]);
+    })
+    .it("invokes oasdiff when spec flag is set to oas");
 });
diff --git a/src/diff/diffCommand.ts b/src/diff/diffCommand.ts
@@ -11,12 +11,11 @@ import { ApiDifferencer } from "./apiDifferencer";
 import { ApiChanges } from "./changes/apiChanges";
 import { ApiCollectionChanges } from "./changes/apiCollectionChanges";
 import { diffRamlDirectories } from "./diffDirectories";
+import { oasDiffChangelog } from "./oasDiff";
 import { allCommonFlags } from "../common/flags";
 import { execSync } from "child_process";
 import fs from "fs-extra";
 
-import { oasDiffChangelog } from "./oasDiff";
-
 export class DiffCommand extends Command {
   // `raml-toolkit --help` only uses the first line, `raml-toolkit diff --help` skips it
   static description = `Compute the difference between two API specifications
@@ -200,37 +199,40 @@ Exit statuses:
     }
   }
 
+  /**
+   * Find the differences between two OAS files.
+   *
+   * Requires oasDiff to be installed to work. Will return an error if oasDiff is not installed
+   *
+   * Otherwise, returns the exit code of oasDiff.oasDiffChangelog
+   *
+   * @param baseApis - Path to a base OAS file
+   * @param newApis - Path to a new OAS file
+   * @param flags - Parsed CLI flags passed to the command
+   */
+  protected async _diffOasFiles(
+    baseApis: string,
+    newApis: string,
+    flags: OutputFlags<typeof DiffCommand.flags>
+  ): Promise<number> {
+    // Diff two files (we do not have a custom ruleset defined for OAS
+    // By default, checks are all 'diff-only'
+    return await oasDiffChangelog(baseApis, newApis, flags);
+  }
+
   async run(): Promise<void> {
     const { args, flags } = this.parse(DiffCommand);
     const baseApis = args.base;
     const newApis = args.new;
-
+    let exitCode = 0;
     if (!(await fs.pathExists(baseApis))) {
-      this.error(`File or directory not found: ${baseApis}`);
+      this.error(`File or directory not found: ${baseApis}`, { exit: 2 });
     }
     if (!(await fs.pathExists(newApis))) {
-      this.error(`File or directory not found: ${newApis}`);
+      this.error(`File or directory not found: ${newApis}`, { exit: 2 });
     }
-
     if (flags.spec === "oas") {
-      // check if oasdiff is installed
-      try {
-        execSync(`oasdiff --version`);
-      } catch (err) {
-        this.error(
-          "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"
-        );
-      }
-
-      if (flags.dir) {
-        const baseApisYamlGlob = '"' + baseApis + "/**/*.yaml" + '"';
-        const newApisYamlGlob = '"' + newApis + "/**/*.yaml" + '"';
-        await oasDiffChangelog(baseApisYamlGlob, newApisYamlGlob, flags);
-      } else {
-        // Diff two files (we do not have a custom ruleset defined for OAS
-        // By default, checks are all 'diff-only'
-        await oasDiffChangelog(baseApis, newApis, flags);
-      }
+      exitCode = await this._diffOasFiles(baseApis, newApis, flags);
     } else {
       if (flags.dir) {
         await this._diffDirs(baseApis, newApis, flags);
@@ -240,6 +242,6 @@ Exit statuses:
         await this._diffFilesUsingRuleset(baseApis, newApis, flags);
       }
     }
-    this.exit();
+    this.exit(exitCode);
   }
 }
diff --git a/src/diff/oasDiff.test.ts b/src/diff/oasDiff.test.ts
@@ -12,7 +12,9 @@ const pq = proxyquire.noCallThru();
 
 describe("oasDiffChangelog", () => {
   it("should execute oasdiff command with correct parameters", async () => {
-    const stub = sinon.stub().returns("");
+    const stub = sinon.stub();
+    stub.onCall(0).returns("version 1.0.0");
+    stub.onCall(1).returns("");
 
     const oasDiff = pq("./oasDiff", {
       child_process: {
@@ -26,12 +28,14 @@ describe("oasDiffChangelog", () => {
     const flags = {};
     const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
 
-    expect(stub.calledOnce).to.be.true;
+    expect(stub.called).to.be.true;
     expect(result).to.equal(0);
   });
 
   it("should return 1 when oasdiff returns a non-empty string", async () => {
-    const execSyncStub = sinon.stub().returns("mock oasdiff change");
+    const execSyncStub = sinon.stub();
+    execSyncStub.onCall(0).returns("version 1.0.0");
+    execSyncStub.onCall(1).returns("mock oasdiff change");
 
     const oasDiff = pq("./oasDiff", {
       child_process: {
@@ -45,12 +49,14 @@ describe("oasDiffChangelog", () => {
     const flags = {};
     const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
 
-    expect(execSyncStub.calledOnce).to.be.true;
+    expect(execSyncStub.called).to.be.true;
     expect(result).to.equal(1);
   });
 
   it("should return 2 when oasdiff throws an error", async () => {
-    const execSyncStub = sinon.stub().throws(new Error("mock oasdiff error"));
+    const execSyncStub = sinon.stub();
+    execSyncStub.onCall(0).returns("version 1.0.0");
+    execSyncStub.onCall(1).throws(new Error("mock oasdiff error"));
 
     const oasDiff = pq("./oasDiff", {
       child_process: {
@@ -64,12 +70,14 @@ describe("oasDiffChangelog", () => {
     const flags = {};
     const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
 
-    expect(execSyncStub.calledOnce).to.be.true;
+    expect(execSyncStub.called).to.be.true;
     expect(result).to.equal(2);
   });
 
   it("should run oasdiff in directory mode when the --dir flag is provided", async () => {
-    const execSyncStub = sinon.stub().returns("a change");
+    const execSyncStub = sinon.stub();
+    execSyncStub.onCall(0).returns("version 1.0.0");
+    execSyncStub.onCall(1).returns("a change");
 
     const oasDiff = pq("./oasDiff", {
       child_process: {
@@ -84,14 +92,16 @@ describe("oasDiffChangelog", () => {
     };
     await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
 
-    expect(execSyncStub.calledOnce).to.be.true;
-    expect(execSyncStub.args[0][0]).to.equal(
-      "oasdiff changelog  --composed base.yaml new.yaml"
+    expect(execSyncStub.called).to.be.true;
+    expect(execSyncStub.args[1][0]).to.equal(
+      'oasdiff changelog  --composed "base.yaml/**/*.yaml" "new.yaml/**/*.yaml"'
     );
   });
 
   it("should save the changes to a file when the --out-file flag is provided", async () => {
-    const execSyncStub = sinon.stub().returns("a change");
+    const execSyncStub = sinon.stub();
+    execSyncStub.onCall(0).returns("version 1.0.0");
+    execSyncStub.onCall(1).returns("a change");
     const fsStub = sinon.stub();
 
     const oasDiff = pq("./oasDiff", {
@@ -111,11 +121,13 @@ describe("oasDiffChangelog", () => {
     };
 
     await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
-    expect(fsStub.calledOnce).to.be.true;
+    expect(fsStub.called).to.be.true;
   });
 
   it("should save the changes to a jsonfile when the --out-file flag is provided and format is json", async () => {
-    const execSyncStub = sinon.stub().returns('{"change": "a change"}');
+    const execSyncStub = sinon.stub();
+    execSyncStub.onCall(0).returns("version 1.0.0");
+    execSyncStub.onCall(1).returns('{"change": "a change"}');
     const fsStub = sinon.stub();
 
     const oasDiff = pq("./oasDiff", {
@@ -136,6 +148,18 @@ describe("oasDiffChangelog", () => {
     };
 
     await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
-    expect(fsStub.calledOnce).to.be.true;
+    expect(fsStub.called).to.be.true;
+  });
+
+  it("should throw an error if oasdiff is not installed", () => {
+    const oasDiff = pq("./oasDiff", {
+      child_process: {
+        execSync: sinon.stub().throws(new Error("oasdiff not installed")),
+      },
+    });
+
+    expect(() => oasDiff.checkOasDiffIsInstalled()).to.throw(
+      "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"
+    );
   });
 });
diff --git a/src/diff/oasDiff.ts b/src/diff/oasDiff.ts
@@ -33,24 +33,34 @@ async function _saveOrLogOas(changes: string, flags): Promise<void> {
 /**
  * Wrapper for oasdiff changelog command.
  *
- * @param baseApi - The base API file
+ * @param baseApi - The base API file or directory
  * @param newApi - The new API file
  * @param flags - Parsed CLI flags passed to the command
  * @returns 0 if no changes are reported, 1 if changes are reported, and 2 if an error occurs
  */
 export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
   try {
+    checkOasDiffIsInstalled();
     console.log("Starting oasdiff");
+
     const jsonMode = flags.format === "json" ? "-f json" : "";
     const directoryMode = flags.dir ? "--composed" : "";
 
+    // If the user is diffing directories, we need to pass the glob pattern to oasdiff
+    let baseApiTarget = baseApi;
+    let newApiTarget = newApi;
+    if (flags.dir) {
+      baseApiTarget = '"' + baseApi + "/**/*.yaml" + '"';
+      newApiTarget = '"' + newApi + "/**/*.yaml" + '"';
+    }
+
     // TODO: Do we want to support the other output formats?
     // See https://github.com/oasdiff/oasdiff/blob/main/docs/BREAKING-CHANGES.md#output-formats
 
     // TODO: Do we want to support customizing severity levels?
     // This would be akin to the raml rulesets
     const oasdiffOutput = execSync(
-      `oasdiff changelog ${jsonMode} ${directoryMode} ${baseApi} ${newApi}`
+      `oasdiff changelog ${jsonMode} ${directoryMode} ${baseApiTarget} ${newApiTarget}`
     ).toString();
 
     if (oasdiffOutput.trim().length === 0) {
@@ -65,3 +75,13 @@ export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
     return 2;
   }
 }
+
+export function checkOasDiffIsInstalled() {
+  try {
+    execSync(`oasdiff --version`).toString();
+  } catch (err) {
+    throw new Error(
+      "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"
+    );
+  }
+}
