diff --git a/src/diff/diffCommand.test.ts b/src/diff/diffCommand.test.ts index 5847347..1989237 100644 --- a/src/diff/diffCommand.test.ts +++ b/src/diff/diffCommand.test.ts @@ -13,7 +13,7 @@ import { Rule } from "json-rules-engine"; import chai from "chai"; import chaiFs from "chai-fs"; -import { DiffCommand as cmd, DiffCommand } from "./diffCommand"; +import { DiffCommand as cmd } from "./diffCommand"; import * as diffDirectories from "./diffDirectories"; import { NodeChanges } from "./changes/nodeChanges"; import { ApiChanges } from "./changes/apiChanges"; @@ -23,11 +23,7 @@ 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" }; diff --git a/src/diff/oasDiff.test.ts b/src/diff/oasDiff.test.ts index 4f77a56..23902a2 100644 --- a/src/diff/oasDiff.test.ts +++ b/src/diff/oasDiff.test.ts @@ -1,165 +1,681 @@ /* - * Copyright (c) 2022, salesforce.com, inc. + * Copyright (c) 2025, salesforce.com, inc. * All rights reserved. * SPDX-License-Identifier: BSD-3-Clause * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ +/* eslint header/header: "off", max-lines:"off" */ + import { expect } from "chai"; import proxyquire from "proxyquire"; import sinon from "sinon"; const pq = proxyquire.noCallThru(); +// Helper functions for test setup +function createMockFs(): { + readdir: sinon.SinonStub; + stat: sinon.SinonStub; + writeFile: sinon.SinonStub; + writeJson: sinon.SinonStub; +} { + return { + readdir: sinon.stub(), + stat: sinon.stub(), + writeFile: sinon.stub(), + writeJson: sinon.stub(), + }; +} + +function createMockExec(): sinon.SinonStub { + const execStub = sinon.stub(); + execStub.callsArgWith(1, null, "version 1.0.0", ""); // Default version check + return execStub; +} + +/** + * Sets up mock directory structure for fs operations in tests. + * + * This helper function configures sinon stubs for fs.readdir and fs.stat to simulate + * a directory tree structure. Files are automatically detected by .json and .yaml extensions. + * + * @param fsStub - The mocked fs object with stubbed readdir and stat methods + * @param structure - Array of directory objects + * + * @example + * // Simple API directory structure + * setupDirectoryStructure(fsStub, [ + * { path: "base", contents: ["api-v1"] }, + * { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] } + * ]); + * + * @example + * // Multiple APIs + * setupDirectoryStructure(fsStub, [ + * { path: "base", contents: ["api-v1", "api-v2"] }, + * { path: "base/api-v1", contents: ["exchange.json"] }, + * { path: "base/api-v2", contents: ["exchange.json"] } + * ]); + * this function equivalant to + * // Manual readdir setup (sequential calls) +* fsStub.readdir.onCall(0).returns(["api-v1"]); // First call: base directory +* fsStub.readdir.onCall(1).returns(["exchange.json", "spec.yaml"]); // Second call: base/api-v1 directory + +* // Manual stat setup for each item +* fsStub.stat.withArgs("base/api-v1").returns({ isDirectory: () => true }); // api-v1 is a directory +* fsStub.stat.withArgs("base/api-v1/exchange.json").returns({ isDirectory: () => false }); // .json is a file +* fsStub.stat.withArgs("base/api-v1/spec.yaml").returns({ isDirectory: () => false }); // .yaml is a file +* // Default behavior - everything else is a directory +* fsStub.stat.returns({ isDirectory: () => true }); + */ +function setupDirectoryStructure( + fsStub: { + readdir: sinon.SinonStub; + stat: sinon.SinonStub; + }, + structure: Array<{ path: string; contents: string[] }> +): void { + for (const dir of structure) { + fsStub.readdir.withArgs(dir.path).resolves(dir.contents); + + for (const item of dir.contents) { + const itemPath = `${dir.path}/${item}`; + const isFile = + item.endsWith(".json") || + item.endsWith(".yaml") || + item.endsWith(".yml"); + fsStub.stat.withArgs(itemPath).resolves({ + isDirectory: () => !isFile, + isFile: () => isFile, + }); + } + } + + fsStub.stat.resolves({ + isDirectory: () => true, + isFile: () => false, + }); +} + +function createOasDiffProxy( + execStub: sinon.SinonStub, + fsStub: { + readdir: sinon.SinonStub; + stat: sinon.SinonStub; + writeFile: sinon.SinonStub; + writeJson: sinon.SinonStub; + } +) { + return pq("./oasDiff", { + child_process: { + exec: execStub, + }, + "fs-extra": fsStub, + }); +} + describe("oasDiffChangelog", () => { - it("should execute oasdiff command with correct parameters", async () => { - const stub = sinon.stub(); - stub.onCall(0).returns("version 1.0.0"); - stub.onCall(1).returns(""); + let consoleErrorSpy; + beforeEach(() => { + consoleErrorSpy = sinon.spy(console, "error"); + }); + afterEach(() => { + consoleErrorSpy.restore(); + }); + it("should return error code 2 when no exchange.json files are found", async () => { + const consoleWarnSpy = sinon.spy(console, "warn"); + const execStub = createMockExec(); + const fsStub = createMockFs(); + + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1"] }, + { path: "base/api-v1", contents: ["spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { dir: true }; + + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(result).to.equal(2); + expect(consoleErrorSpy.called).to.be.true; + expect(consoleErrorSpy.args[0][0].message).to.include( + "No exchange.json files found in any directory under: base" + ); + + consoleWarnSpy.restore(); + }); + + it("should return error code 2 when no exchange.json files are found in entire directory tree", async () => { + const execStub = createMockExec(); + const fsStub = createMockFs(); + + setupDirectoryStructure(fsStub, [{ path: "base", contents: [] }]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { dir: true }; + + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(result).to.equal(2); + expect(consoleErrorSpy.called).to.be.true; + expect(consoleErrorSpy.args[0][0].message).to.include( + "No exchange.json files found in any directory under: base. Each API directory must contain an exchange.json file." + ); + }); + + it("should return error code 2 when maximum directory depth is exceeded", async () => { + const execStub = createMockExec(); + const fsStub = createMockFs(); + + // Create very deep directory structure (4 levels deep, exceeding limit of 3) + const deepStructure: Array<{ path: string; contents: string[] }> = []; + let currentPath = "base"; + for (let i = 0; i <= 4; i++) { + deepStructure.push({ path: currentPath, contents: ["nested"] }); + currentPath += "/nested"; + } + + setupDirectoryStructure(fsStub, deepStructure); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { dir: true }; + + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(result).to.equal(2); + expect(consoleErrorSpy.called).to.be.true; + expect(consoleErrorSpy.args[0][0].message).to.include( + "Maximum directory depth (3) exceeded while searching for exchange.json files in: base/nested/nested/nested/nested" + ); + }); + + it("should throw an error if oasdiff is not installed", async () => { + const execStub = sinon.stub(); + execStub.callsArgWith(1, new Error("oasdiff not installed")); const oasDiff = pq("./oasDiff", { child_process: { - execSync: stub, + exec: execStub, }, }); - // Arrange - const baseApi = "base.yaml"; - const newApi = "new.yaml"; + try { + await oasDiff.checkOasDiffIsInstalled(); + expect.fail("Expected function to throw an error"); + } catch (error) { + expect(error.message).to.equal( + "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation" + ); + } + }); + + it("should return 2 when oasdiff throws an error", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, new Error("mock oasdiff error")); + + const fsStub = createMockFs(); + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; const flags = {}; const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(stub.called).to.be.true; - expect(result).to.equal(0); + expect(execStub.called).to.be.true; + expect(result).to.equal(2); }); - it("should return 1 when oasdiff returns a non-empty string", async () => { - const execSyncStub = sinon.stub(); - execSyncStub.onCall(0).returns("version 1.0.0"); - execSyncStub.onCall(1).returns("mock oasdiff change"); + it("should execute oasdiff command with correct parameters for single file mode", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "", ""); // diff result - const oasDiff = pq("./oasDiff", { - child_process: { - execSync: execSyncStub, - }, - }); + const fsStub = createMockFs(); + const oasDiff = createOasDiffProxy(execStub, fsStub); - // Arrange const baseApi = "base.yaml"; const newApi = "new.yaml"; const flags = {}; const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(execSyncStub.called).to.be.true; - expect(result).to.equal(1); + expect(execStub.called).to.be.true; + expect(execStub.args[1][0]).to.equal( + 'oasdiff changelog "base.yaml" "new.yaml"' + ); + expect(result).to.equal(0); }); - it("should return 2 when oasdiff throws an error", async () => { - const execSyncStub = sinon.stub(); - execSyncStub.onCall(0).returns("version 1.0.0"); - execSyncStub.onCall(1).throws(new Error("mock oasdiff error")); + it("should execute oasdiff command with correct parameters for directory mode", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "", ""); // diff result - const oasDiff = pq("./oasDiff", { - child_process: { - execSync: execSyncStub, - }, - }); + 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"] }, + ]); - // Arrange - const baseApi = "base.yaml"; - const newApi = "new.yaml"; + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { dir: true }; + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(execStub.called).to.be.true; + expect(execStub.args[1][0]).to.equal( + 'oasdiff changelog "base/api-v1/spec.yaml" "new/api-v1/spec.yaml"' + ); + expect(result).to.equal(0); + }); + + it("should return 1 when oasdiff returns a non-empty string", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "mock oasdiff change", ""); + + const fsStub = createMockFs(); + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; const flags = {}; const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(execSyncStub.called).to.be.true; - expect(result).to.equal(2); + expect(execStub.called).to.be.true; + expect(result).to.equal(1); }); it("should run oasdiff in directory mode when the --dir flag is provided", async () => { - const execSyncStub = sinon.stub(); - execSyncStub.onCall(0).returns("version 1.0.0"); - execSyncStub.onCall(1).returns("a change"); + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "a minor change", ""); - const oasDiff = pq("./oasDiff", { - child_process: { - execSync: execSyncStub, - }, - }); + 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"] }, + ]); - const baseApi = "base.yaml"; - const newApi = "new.yaml"; + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; const flags = { dir: true, }; await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(execSyncStub.called).to.be.true; - expect(execSyncStub.args[1][0]).to.equal( - 'oasdiff changelog --composed "base.yaml/**/*.yaml" "new.yaml/**/*.yaml"' + expect(execStub.called).to.be.true; + expect(execStub.args[1][0]).to.equal( + 'oasdiff changelog "base/api-v1/spec.yaml" "new/api-v1/spec.yaml"' ); }); - it("should save the changes to a file when the --out-file flag is provided", async () => { - const execSyncStub = sinon.stub(); - execSyncStub.onCall(0).returns("version 1.0.0"); - execSyncStub.onCall(1).returns("a change"); - const fsStub = sinon.stub(); + it("should concatenate results from multiple directories in text format", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "changes in api-v1", ""); + execStub.onThirdCall().callsArgWith(1, null, "changes in api-v2", ""); - const oasDiff = pq("./oasDiff", { - child_process: { - execSync: execSyncStub, - }, - "fs-extra": { - writeFile: fsStub, - }, + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/api-v2", contents: ["exchange.json", "spec.yaml"] }, + { path: "new", contents: ["api-v1", "api-v2"] }, + { path: "new/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/api-v2", contents: ["exchange.json", "spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.txt", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeFile.called).to.be.true; + const writtenContent = fsStub.writeFile.args[0][1]; + expect(writtenContent).to.include("=== Changes in api-v1 ==="); + expect(writtenContent).to.include("changes in api-v1"); + expect(writtenContent).to.include("=== Changes in api-v2 ==="); + expect(writtenContent).to.include("changes in api-v2"); + }); + + it("should concatenate results from multiple directories in JSON format", async () => { + const execStub = createMockExec(); + execStub + .onSecondCall() + .callsArgWith(1, null, '[{"changes": "in api-v1"}]', ""); + execStub + .onThirdCall() + .callsArgWith(1, null, '[{"changes": "in api-v2"}]', ""); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/api-v2", contents: ["exchange.json", "spec.yaml"] }, + { path: "new", contents: ["api-v1", "api-v2"] }, + { path: "new/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/api-v2", contents: ["exchange.json", "spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.json", + format: "json", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeJson.called).to.be.true; + const writtenContent = fsStub.writeJson.args[0][1]; + expect(writtenContent).to.be.an("array").with.lengthOf(2); + expect(writtenContent[0]).to.deep.equal({ + directory: "api-v1", + changes: [{ changes: "in api-v1" }], + }); + expect(writtenContent[1]).to.deep.equal({ + directory: "api-v2", + changes: [{ changes: "in api-v2" }], }); + }); - // Arrange - const baseApi = "base.yaml"; - const newApi = "new.yaml"; + it("should report deleted APIs when directories exist in base but not in new", async () => { + const execStub = createMockExec(); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json"] }, + { path: "base/api-v2", contents: ["exchange.json"] }, + { path: "new", contents: ["api-v2"] }, // only api-v2 + { path: "new/api-v2", contents: ["exchange.json"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; const flags = { "out-file": "output.txt", + dir: true, }; await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(fsStub.called).to.be.true; + + expect(fsStub.writeFile.called).to.be.true; + const writtenContent = fsStub.writeFile.args[0][1]; + expect(writtenContent).to.include("======api-v1 API is deleted======"); }); - it("should save the changes to a jsonfile when the --out-file flag is provided and format is json", async () => { - const execSyncStub = sinon.stub(); - execSyncStub.onCall(0).returns("version 1.0.0"); - execSyncStub.onCall(1).returns('{"change": "a change"}'); - const fsStub = sinon.stub(); + it("should report added APIs when directories exist in new but not in base", async () => { + const execStub = createMockExec(); - const oasDiff = pq("./oasDiff", { - child_process: { - execSync: execSyncStub, - }, - "fs-extra": { - writeJson: fsStub, - }, + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1"] }, + { path: "base/api-v1", contents: ["exchange.json"] }, + { path: "new", contents: ["api-v1", "api-v2"] }, + { path: "new/api-v1", contents: ["exchange.json"] }, + { path: "new/api-v2", contents: ["exchange.json"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.txt", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeFile.called).to.be.true; + const writtenContent = fsStub.writeFile.args[0][1]; + expect(writtenContent).to.include("======api-v2 API is added======"); + }); + + it("should report both added and deleted APIs in the same comparison", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "changes in api-v2", ""); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/api-v2", contents: ["exchange.json", "spec.yaml"] }, + { path: "new", contents: ["api-v2", "api-v3"] }, + { path: "new/api-v2", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/api-v3", contents: ["exchange.json", "spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.txt", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeFile.called).to.be.true; + const writtenContent = fsStub.writeFile.args[0][1]; + expect(writtenContent).to.include("======api-v1 API is deleted======"); + expect(writtenContent).to.include("======api-v3 API is added======"); + expect(writtenContent).to.include("=== Changes in api-v2 ==="); + }); + + it("should handle mixed scenarios with changes, additions, and deletions", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "changes in common-api", ""); + execStub.onThirdCall().callsArgWith(1, null, "", ""); // no changes in stable-api + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["common-api", "stable-api", "old-api"] }, + { path: "base/common-api", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/stable-api", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/old-api", contents: ["exchange.json", "spec.yaml"] }, + { path: "new", contents: ["common-api", "stable-api", "new-api"] }, + { path: "new/common-api", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/stable-api", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/new-api", contents: ["exchange.json", "spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.txt", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeFile.called).to.be.true; + const writtenContent = fsStub.writeFile.args[0][1]; + + // Should show deleted API + expect(writtenContent).to.include("======old-api API is deleted======"); + + // Should show added API + expect(writtenContent).to.include("======new-api API is added======"); + + // Should show changes in common-api + expect(writtenContent).to.include("=== Changes in common-api ==="); + expect(writtenContent).to.include("changes in common-api"); + + // Should NOT show stable-api since it has no changes + expect(writtenContent).to.not.include("=== Changes in stable-api ==="); + }); + + it("should report deleted APIs in JSON format", async () => { + const execStub = createMockExec(); + // Empty JSON array (no changes in api-v2) + execStub.onSecondCall().callsArgWith(1, null, "[]", ""); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json"] }, + { path: "base/api-v2", contents: ["exchange.json"] }, + { path: "new", contents: ["api-v2"] }, // only api-v2 + { path: "new/api-v2", contents: ["exchange.json"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.json", + format: "json", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeJson.called).to.be.true; + const writtenContent = fsStub.writeJson.args[0][1]; + expect(writtenContent).to.be.an("array").with.lengthOf(1); + expect(writtenContent[0]).to.deep.equal({ + directory: "api-v1", + status: "deleted", + message: "api-v1 API is deleted", }); + }); - // Arrange - const baseApi = "base.yaml"; - const newApi = "new.yaml"; + it("should report added APIs in JSON format", async () => { + const execStub = createMockExec(); + // Empty JSON array (no changes in api-v1) + execStub.onSecondCall().callsArgWith(1, null, "[]", ""); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1"] }, + { path: "base/api-v1", contents: ["exchange.json"] }, + { path: "new", contents: ["api-v1", "api-v2"] }, + { path: "new/api-v1", contents: ["exchange.json"] }, + { path: "new/api-v2", contents: ["exchange.json"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; const flags = { "out-file": "output.json", format: "json", + dir: true, }; await oasDiff.oasDiffChangelog(baseApi, newApi, flags); - expect(fsStub.called).to.be.true; + + expect(fsStub.writeJson.called).to.be.true; + const writtenContent = fsStub.writeJson.args[0][1]; + expect(writtenContent).to.be.an("array").with.lengthOf(1); + expect(writtenContent[0]).to.deep.equal({ + directory: "api-v2", + status: "added", + message: "api-v2 API is added", + }); }); - 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")), - }, + it("should not include directories with empty changes in JSON format", async () => { + const execStub = createMockExec(); + execStub + .onSecondCall() + .callsArgWith(1, null, '[{"changes": "in api-v1"}]', ""); + execStub.onThirdCall().callsArgWith(1, null, "[]", ""); + + const fsStub = createMockFs(); + setupDirectoryStructure(fsStub, [ + { path: "base", contents: ["api-v1", "api-v2"] }, + { path: "base/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "base/api-v2", contents: ["exchange.json", "spec.yaml"] }, + { path: "new", contents: ["api-v1", "api-v2"] }, + { path: "new/api-v1", contents: ["exchange.json", "spec.yaml"] }, + { path: "new/api-v2", contents: ["exchange.json", "spec.yaml"] }, + ]); + + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base"; + const newApi = "new"; + const flags = { + "out-file": "output.json", + format: "json", + dir: true, + }; + + await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(fsStub.writeJson.called).to.be.true; + const writtenContent = fsStub.writeJson.args[0][1]; + expect(writtenContent).to.be.an("array").with.lengthOf(1); + expect(writtenContent[0]).to.deep.equal({ + directory: "api-v1", + changes: [{ changes: "in api-v1" }], }); + }); - expect(() => oasDiff.checkOasDiffIsInstalled()).to.throw( - "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation" - ); + it("should not include empty results in single file JSON mode", async () => { + const execStub = createMockExec(); + execStub.onSecondCall().callsArgWith(1, null, "[]", ""); + + const fsStub = createMockFs(); + const oasDiff = createOasDiffProxy(execStub, fsStub); + + const baseApi = "base.yaml"; + const newApi = "new.yaml"; + const flags = { format: "json" }; + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(execStub.called).to.be.true; + expect(result).to.equal(0); + }); + + it("should include non-empty results in single file JSON mode", async () => { + const execStub = createMockExec(); + execStub + .onSecondCall() + .callsArgWith(1, null, '[{"change": "something"}]', ""); // non-empty array result + + const fsStub = createMockFs(); + const oasDiff = createOasDiffProxy(execStub, fsStub); + + // Arrange + const baseApi = "base.yaml"; + const newApi = "new.yaml"; + const flags = { format: "json" }; + const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags); + + expect(execStub.called).to.be.true; + expect(result).to.equal(1); // Changes should be reported }); }); diff --git a/src/diff/oasDiff.ts b/src/diff/oasDiff.ts index 8c05561..19c0976 100644 --- a/src/diff/oasDiff.ts +++ b/src/diff/oasDiff.ts @@ -4,8 +4,183 @@ * SPDX-License-Identifier: BSD-3-Clause * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ +/* eslint header/header: "off", max-lines:"off" */ + import fs from "fs-extra"; -import { execSync } from "child_process"; +import { exec } from "child_process"; +import path from "path"; + +/** + * Interface for flags used in oasDiff functions + */ +interface OasDiffFlags { + format?: "json" | "console"; + dir?: boolean; + "out-file"?: string; +} + +/** + * Represents a directory-level API change (added/deleted) + */ +interface DirectoryStatusChange { + directory: string; + status: "added" | "deleted"; + message: string; +} + +/** + * Represents file-level changes within a directory + */ +interface FileChange { + file: string; + status: "added" | "deleted"; + message: string; +} + +/** + * Represents changes found in a directory with detailed change information + */ +interface DirectoryChanges { + directory: string; + changes: (FileChange | Record)[]; +} + +/** + * Union type for all possible result items + * - string: Text format output + * - DirectoryStatusChange: JSON format for added/deleted directories + * - DirectoryChanges: JSON format for directories with file changes + * - unknown[]: Parsed JSON output from oasdiff (structure depends on oasdiff output) + */ +type OasDiffResult = + | string + | DirectoryStatusChange + | DirectoryChanges + | unknown[]; + +/** + * Return type for oasDiff handler functions + */ +interface OasDiffHandlerResult { + results: OasDiffResult[]; + hasChanges: boolean; + hasErrors: boolean; +} + +/** + * Recursively find directories containing exchange.json files + * + * @param rootPath - The root path to search from + * @returns Array of objects with directory name and full path + */ +async function findExchangeDirectories( + rootPath: string +): Promise> { + const result = []; + // assume the directory depth is 3, we don't want to go too deep + const maxDepth = 3; + let foundAnyExchangeJson = false; + + async function searchDirectory(currentPath: string, depth = 0) { + if (depth > maxDepth) { + throw new Error( + `Maximum directory depth (${maxDepth}) exceeded while searching for exchange.json files in: ${currentPath}` + ); + } + + try { + const entries = await fs.readdir(currentPath); + + // Check if current directory contains exchange.json + const hasExchangeJson = entries.includes("exchange.json"); + + if (hasExchangeJson) { + foundAnyExchangeJson = true; + const dirName = path.basename(currentPath); + result.push({ + name: dirName, + path: currentPath, + }); + return; + } + + // Check if this is a leaf directory (no subdirectories) + const subdirectories = []; + for (const entry of entries) { + const entryPath = path.join(currentPath, entry); + const stat = await fs.stat(entryPath); + if (stat.isDirectory()) { + subdirectories.push(entryPath); + } + } + // If this is a leaf directory and we haven't found exchange.json, that's an error + if (subdirectories.length === 0 && !hasExchangeJson) { + console.warn( + `No exchange.json file found in leaf directory: ${currentPath}. Each API directory must contain an exchange.json file.` + ); + } + + // Search subdirectories + for (const subPath of subdirectories) { + await searchDirectory(subPath, depth + 1); + } + } catch (err) { + // Re-throw our custom errors + if ( + err.message.includes("exchange.json") || + err.message.includes("Maximum directory depth") + ) { + throw err; + } + // Ignore other errors for individual directories (permissions, etc.) + console.warn( + `Warning: Could not read directory ${currentPath}:`, + err.message + ); + } + } + + await searchDirectory(rootPath); + + // Final check: if we didn't find any exchange.json files at all + if (!foundAnyExchangeJson) { + throw new Error( + `No exchange.json files found in any directory under: ${rootPath}. Each API directory must contain an exchange.json file.` + ); + } + + return result; +} + +/** + * Find YAML files in a directory + * + * @param dirPath - The directory path to search + * @returns Array of YAML file paths + */ +async function findYamlFiles(dirPath: string): Promise { + try { + const entries = await fs.readdir(dirPath); + const yamlFiles = []; + + for (const entry of entries) { + const entryPath = path.join(dirPath, entry); + const stat = await fs.stat(entryPath); + + if ( + stat.isFile() && + (entry.endsWith(".yaml") || entry.endsWith(".yml")) + ) { + yamlFiles.push(entryPath); + } + } + + return yamlFiles; + } catch (err) { + console.warn(`Warning: Could not read directory ${dirPath}:`, err.message); + return []; + } +} /** * If a file is given, saves the changes to the file, as JSON by default. @@ -14,15 +189,18 @@ import { execSync } from "child_process"; * @param changes - The changes to save or log * @param flags - Parsed CLI flags passed to the command */ -async function _saveOrLogOas(changes: string, flags): Promise { +async function _saveOrLogOas( + changes: string, + flags: OasDiffFlags +): Promise { const file = flags["out-file"]; if (file) { - console.log(`API Changes found! Saving results to file ${file}`); + console.log(`API Changes found! Saving results to file ${file}:`); if (flags.format === "json") { - console.log("Using json format"); + console.log(` using json format`); await fs.writeJson(file, JSON.parse(changes)); } else { - console.log("Using console format"); + console.log(` using console format`); await fs.writeFile(file, changes); } } else { @@ -30,55 +208,308 @@ async function _saveOrLogOas(changes: string, flags): Promise { } } +/** + * Execute oasdiff changelog command + * + * @param baseSpec - The base spec path + * @param newSpec - The new spec path + * @param jsonMode - JSON format flag + * @param directoryMode - Directory mode flag + * @returns The stdout output from oasdiff + */ +async function executeOasDiff( + baseSpec: string, + newSpec: string, + jsonMode: string, + directoryMode = "" +): Promise { + return new Promise((resolve, reject) => { + const flags = [jsonMode, directoryMode] + .filter((flag) => flag.trim() !== "") + .join(" "); + const flagsString = flags ? ` ${flags}` : ""; + // intentionally not leaving space for flagsString + exec( + `oasdiff changelog${flagsString} "${baseSpec}" "${newSpec}"`, + (error, stdout) => { + if (error) { + reject(error); + } else { + resolve(stdout); + } + } + ); + }); +} + +/** + * Handle directory mode comparison logic + * + * @param baseApi - The base API directory + * @param newApi - The new API directory + * @param jsonMode - JSON formatting flag + * @param flags - CLI flags + * @returns Object containing results, hasChanges flag, and hasErrors flag + */ +async function handleDirectoryMode( + baseApi: string, + newApi: string, + jsonMode: string, + flags: OasDiffFlags +): Promise { + const allResults: OasDiffResult[] = []; + let hasChanges = false; + let hasErrors = false; + + // Find all exchange.json files and their parent directories + const baseExchangeDirs = await findExchangeDirectories(baseApi); + const newExchangeDirs = await findExchangeDirectories(newApi); + + const allDirNames = new Set([ + ...baseExchangeDirs.map((dir) => dir.name), + ...newExchangeDirs.map((dir) => dir.name), + ]); + + for (const dirName of allDirNames) { + const baseDir = baseExchangeDirs.find((dir) => dir.name === dirName); + const newDir = newExchangeDirs.find((dir) => dir.name === dirName); + + // Check if directory was deleted + if (baseDir && !newDir) { + console.log(`${dirName} API is deleted`); + if (flags.format === "json") { + allResults.push({ + directory: dirName, + status: "deleted", + message: `${dirName} API is deleted`, + }); + } else { + allResults.push(`======${dirName} API is deleted======`); + } + hasChanges = true; + continue; + } + + // Check if directory was added + if (!baseDir && newDir) { + console.log(`${dirName} API is added`); + if (flags.format === "json") { + allResults.push({ + directory: dirName, + status: "added", + message: `${dirName} API is added`, + }); + } else { + allResults.push(`======${dirName} API is added======`); + } + hasChanges = true; + continue; + } + + // Both directories exist, compare individual yml files in each directory + if (baseDir && newDir) { + console.log("==================================="); + console.log(`Processing directory pair: ${dirName}`); + + try { + const baseYamlFiles = await findYamlFiles(baseDir.path); + const newYamlFiles = await findYamlFiles(newDir.path); + + const directoryChanges: (FileChange | Record)[] = []; + const directoryChangesText: string[] = []; + + // Process each YAML file pair + for (const baseYamlFile of baseYamlFiles) { + const baseFileName = path.basename(baseYamlFile); + const newYamlFile = newYamlFiles.find( + (f) => path.basename(f) === baseFileName + ); + + if (newYamlFile) { + console.log(`Comparing ${baseFileName} in ${dirName}`); + const oasdiffOutput = await executeOasDiff( + baseYamlFile, + newYamlFile, + jsonMode + ); + + if (oasdiffOutput.trim().length > 0) { + if (flags.format === "json") { + const outputJson = JSON.parse(oasdiffOutput); + if (outputJson?.length > 0) { + directoryChanges.push(...outputJson); + } + } else { + directoryChangesText.push( + `--- Changes in ${baseFileName} ---\n${oasdiffOutput}` + ); + } + } + } else { + console.log(`File ${baseFileName} was deleted in ${dirName}`); + if (flags.format === "json") { + directoryChanges.push({ + file: baseFileName, + status: "deleted", + message: `File ${baseFileName} was deleted`, + }); + } else { + directoryChangesText.push( + `--- File ${baseFileName} was deleted ---` + ); + } + } + } + + // Check for added files + for (const newYamlFile of newYamlFiles) { + const newFileName = path.basename(newYamlFile); + const baseYamlFile = baseYamlFiles.find( + (f) => path.basename(f) === newFileName + ); + + if (!baseYamlFile) { + console.log(`File ${newFileName} was added in ${dirName}`); + if (flags.format === "json") { + directoryChanges.push({ + file: newFileName, + status: "added", + message: `File ${newFileName} was added`, + }); + } else { + directoryChangesText.push( + `--- File ${newFileName} was added ---` + ); + } + } + } + + if (directoryChanges.length > 0 || directoryChangesText.length > 0) { + console.log(`Changes found in ${dirName}`); + if (flags.format === "json") { + allResults.push({ + directory: dirName, + changes: directoryChanges, + }); + } else { + const formattedOutput = `=== Changes in ${dirName} ===\n${directoryChangesText.join( + "\n" + )}`; + allResults.push(formattedOutput); + } + hasChanges = true; + } else { + console.log(`No changes found in ${dirName}`); + } + } catch (err) { + console.error(`Error processing ${dirName}:`, err); + hasErrors = true; + } + } + } + + return { results: allResults, hasChanges, hasErrors }; +} + +/** + * Handle single file mode comparison logic + * + * @param baseApi - The base API file + * @param newApi - The new API file + * @param jsonMode - JSON formatting flag + * @param flags - CLI flags + * @returns Object containing results, hasChanges flag, and hasErrors flag + */ +async function handleSingleFileMode( + baseApi: string, + newApi: string, + jsonMode: string, + flags: OasDiffFlags +): Promise { + const allResults: OasDiffResult[] = []; + let hasChanges = false; + let hasErrors = false; + + try { + const oasdiffOutput = await executeOasDiff(baseApi, newApi, jsonMode); + + if (oasdiffOutput.trim().length > 0) { + console.log("Changes found"); + if (flags.format === "json") { + // For JSON format, parse the output + const outputJson = JSON.parse(oasdiffOutput); + if (outputJson?.length > 0) { + allResults.push(outputJson); + hasChanges = true; + } + } else { + allResults.push(oasdiffOutput); + hasChanges = true; + } + } else { + console.log("No changes found"); + } + } catch (err) { + console.error("Error processing files:", err); + hasErrors = true; + } + + return { results: allResults, hasChanges, hasErrors }; +} + /** * Wrapper for oasdiff changelog command. * * @param baseApi - The base API file or directory - * @param newApi - The new API file + * @param newApi - The new API file or directory * @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) { +export async function oasDiffChangelog( + baseApi: string, + newApi: string, + flags: OasDiffFlags +) { try { - checkOasDiffIsInstalled(); - console.log("Starting oasdiff"); + await 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 + // Handle directory mode or single file mode + const { results, hasChanges, hasErrors } = flags.dir + ? await handleDirectoryMode(baseApi, newApi, jsonMode, flags) + : await handleSingleFileMode(baseApi, newApi, jsonMode, flags); - // TODO: Do we want to support customizing severity levels? - // This would be akin to the raml rulesets - const oasdiffOutput = execSync( - `oasdiff changelog ${jsonMode} ${directoryMode} ${baseApiTarget} ${newApiTarget}` - ).toString(); + if (hasChanges) { + if (flags.format === "json") { + await _saveOrLogOas(JSON.stringify(results, null, 2), flags); + } else { + await _saveOrLogOas(results.join("\n"), flags); + } + } - if (oasdiffOutput.trim().length === 0) { - console.log("No API changes reported by oasdiff"); - return 0; - } else { - await _saveOrLogOas(oasdiffOutput, flags); - return 1; + if (hasErrors) { + return 2; } + return hasChanges ? 1 : 0; } catch (err) { console.error(err); return 2; } } -export function checkOasDiffIsInstalled() { +export async function checkOasDiffIsInstalled() { try { - execSync(`oasdiff --version`).toString(); + await new Promise((resolve, reject) => { + exec(`oasdiff --version`, (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }); + }); } catch (err) { throw new Error( "oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation" diff --git a/src/generate-from-oas/generateCommand.ts b/src/generate-from-oas/generateCommand.ts index dd05d62..9e45f41 100644 --- a/src/generate-from-oas/generateCommand.ts +++ b/src/generate-from-oas/generateCommand.ts @@ -10,7 +10,6 @@ import { allCommonFlags } from "../common/flags"; import { generateFromOas, DEFAULT_CONFIG_PACKAGE_PATH, - DEFAULT_CONFIG_PATH, } from "./generateFromOas"; export class GenerateCommand extends Command { diff --git a/src/generate-from-oas/generateFromOas.ts b/src/generate-from-oas/generateFromOas.ts index 1d7609e..874ddb4 100644 --- a/src/generate-from-oas/generateFromOas.ts +++ b/src/generate-from-oas/generateFromOas.ts @@ -7,7 +7,6 @@ import path from "path"; import { execSync } from "child_process"; -import { string, boolean } from "@oclif/parser/lib/flags"; // Path relative to project root export const DEFAULT_CONFIG_BASE_PATH =