Skip to content

Commit 37c1632

Browse files
alexvuongalexvuong
committed
PR feedback
1 parent d1b318d commit 37c1632

File tree

2 files changed

+234
-18
lines changed

2 files changed

+234
-18
lines changed

‎src/diff/oasDiff.test.ts‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,176 @@ describe("oasDiffChangelog", () => {
261261
expect(execSyncStub.callCount).to.equal(2);
262262
});
263263

264+
it("should report deleted APIs when directories exist in base but not in new", async () => {
265+
const execSyncStub = sinon.stub();
266+
execSyncStub.onCall(0).returns("version 1.0.0");
267+
268+
const fsStub = {
269+
readdir: sinon.stub(),
270+
stat: sinon.stub(),
271+
writeFile: sinon.stub(),
272+
};
273+
274+
// Base has api-v1 and api-v2, new only has api-v1
275+
fsStub.readdir.onCall(0).returns(["api-v1", "api-v2"]); // base directories
276+
fsStub.readdir.onCall(1).returns(["api-v2"]); // new directories
277+
278+
// All stat calls return isDirectory true
279+
fsStub.stat.returns({ isDirectory: () => true });
280+
281+
const oasDiff = pq("./oasDiff", {
282+
child_process: {
283+
execSync: execSyncStub,
284+
},
285+
"fs-extra": fsStub,
286+
});
287+
288+
const baseApi = "base";
289+
const newApi = "new";
290+
const flags = {
291+
"out-file": "output.txt",
292+
dir: true,
293+
};
294+
295+
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
296+
297+
expect(fsStub.writeFile.called).to.be.true;
298+
const writtenContent = fsStub.writeFile.args[0][1];
299+
expect(writtenContent).to.include("======api-v1 API is deleted======");
300+
});
301+
302+
it("should report added APIs when directories exist in new but not in base", async () => {
303+
const execSyncStub = sinon.stub();
304+
execSyncStub.onCall(0).returns("version 1.0.0");
305+
306+
const fsStub = {
307+
readdir: sinon.stub(),
308+
stat: sinon.stub(),
309+
writeFile: sinon.stub(),
310+
};
311+
312+
// Base has only api-v1, new has api-v1 and api-v2
313+
fsStub.readdir.onCall(0).returns(["api-v1"]); // base directories
314+
fsStub.readdir.onCall(1).returns(["api-v1", "api-v2"]); // new directories
315+
316+
// All stat calls return isDirectory true
317+
fsStub.stat.returns({ isDirectory: () => true });
318+
319+
const oasDiff = pq("./oasDiff", {
320+
child_process: {
321+
execSync: execSyncStub,
322+
},
323+
"fs-extra": fsStub,
324+
});
325+
326+
const baseApi = "base";
327+
const newApi = "new";
328+
const flags = {
329+
"out-file": "output.txt",
330+
dir: true,
331+
};
332+
333+
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
334+
335+
expect(fsStub.writeFile.called).to.be.true;
336+
const writtenContent = fsStub.writeFile.args[0][1];
337+
expect(writtenContent).to.include("======api-v2 API is added======");
338+
});
339+
340+
it("should report both added and deleted APIs in the same comparison", async () => {
341+
const execSyncStub = sinon.stub();
342+
execSyncStub.onCall(0).returns("version 1.0.0");
343+
execSyncStub.onCall(1).returns("changes in api-v1");
344+
345+
const fsStub = {
346+
readdir: sinon.stub(),
347+
stat: sinon.stub(),
348+
writeFile: sinon.stub(),
349+
};
350+
351+
// Base has api-v1 and api-v2, new has api-v1 and api-v3
352+
fsStub.readdir.onCall(0).returns(["api-v1", "api-v2"]); // base directories
353+
fsStub.readdir.onCall(1).returns(["api-v2", "api-v3"]); // new directories
354+
355+
// All stat calls return isDirectory true
356+
fsStub.stat.returns({ isDirectory: () => true });
357+
358+
const oasDiff = pq("./oasDiff", {
359+
child_process: {
360+
execSync: execSyncStub,
361+
},
362+
"fs-extra": fsStub,
363+
});
364+
365+
const baseApi = "base";
366+
const newApi = "new";
367+
const flags = {
368+
"out-file": "output.txt",
369+
dir: true,
370+
};
371+
372+
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
373+
374+
expect(fsStub.writeFile.called).to.be.true;
375+
const writtenContent = fsStub.writeFile.args[0][1];
376+
expect(writtenContent).to.include("======api-v1 API is deleted======");
377+
expect(writtenContent).to.include("======api-v3 API is added======");
378+
expect(writtenContent).to.include("=== Changes in api-v2 ===");
379+
});
380+
381+
it("should handle mixed scenarios with changes, additions, and deletions", async () => {
382+
const execSyncStub = sinon.stub();
383+
execSyncStub.onCall(0).returns("version 1.0.0");
384+
execSyncStub.onCall(1).returns("changes in common-api");
385+
execSyncStub.onCall(2).returns(""); // no changes in stable-api
386+
387+
const fsStub = {
388+
readdir: sinon.stub(),
389+
stat: sinon.stub(),
390+
writeFile: sinon.stub(),
391+
};
392+
393+
// Base: common-api, stable-api, old-api
394+
// New: common-api, stable-api, new-api
395+
fsStub.readdir.onCall(0).returns(["common-api", "stable-api", "old-api"]); // base
396+
fsStub.readdir.onCall(1).returns(["common-api", "stable-api", "new-api"]); // new
397+
398+
// All stat calls return isDirectory true
399+
fsStub.stat.returns({ isDirectory: () => true });
400+
401+
const oasDiff = pq("./oasDiff", {
402+
child_process: {
403+
execSync: execSyncStub,
404+
},
405+
"fs-extra": fsStub,
406+
});
407+
408+
const baseApi = "base";
409+
const newApi = "new";
410+
const flags = {
411+
"out-file": "output.txt",
412+
dir: true,
413+
};
414+
415+
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
416+
417+
expect(fsStub.writeFile.called).to.be.true;
418+
const writtenContent = fsStub.writeFile.args[0][1];
419+
420+
// Should show deleted API
421+
expect(writtenContent).to.include("======old-api API is deleted======");
422+
423+
// Should show added API
424+
expect(writtenContent).to.include("======new-api API is added======");
425+
426+
// Should show changes in common-api
427+
expect(writtenContent).to.include("=== Changes in common-api ===");
428+
expect(writtenContent).to.include("changes in common-api");
429+
430+
// Should NOT show stable-api since it has no changes
431+
expect(writtenContent).to.not.include("=== Changes in stable-api ===");
432+
});
433+
264434
it("should throw an error if oasdiff is not installed", () => {
265435
const oasDiff = pq("./oasDiff", {
266436
child_process: {

‎src/diff/oasDiff.ts‎

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
66
*/
77
import fs from "fs-extra";
8-
import { execSync } from "child_process";
8+
import { exec } from "child_process";
9+
import { promisify } from "util";
10+
11+
const execAsync = promisify(exec);
912

1013
/**
1114
* If a file is given, saves the changes to the file, as JSON by default.
@@ -30,6 +33,27 @@ async function _saveOrLogOas(changes: string, flags): Promise<void> {
3033
}
3134
}
3235

36+
/**
37+
* Execute oasdiff changelog command
38+
*
39+
* @param baseSpec - The base spec path
40+
* @param newSpec - The new spec path
41+
* @param jsonMode - JSON format flag
42+
* @param directoryMode - Directory mode flag
43+
* @returns The stdout output from oasdiff
44+
*/
45+
async function executeOasDiff(
46+
baseSpec: string,
47+
newSpec: string,
48+
jsonMode: string,
49+
directoryMode = ""
50+
): Promise<string> {
51+
const { stdout } = await execAsync(
52+
`oasdiff changelog ${jsonMode} ${directoryMode} "${baseSpec}" "${newSpec}"`
53+
);
54+
return stdout;
55+
}
56+
3357
/**
3458
* Wrapper for oasdiff changelog command.
3559
*
@@ -40,7 +64,7 @@ async function _saveOrLogOas(changes: string, flags): Promise<void> {
4064
*/
4165
export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
4266
try {
43-
checkOasDiffIsInstalled();
67+
await checkOasDiffIsInstalled();
4468
console.log("......Starting oasdiff......");
4569

4670
const jsonMode = flags.format === "json" ? "-f json" : "";
@@ -61,23 +85,31 @@ export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
6185
const baseDirPath = `${baseApi}/${baseDir}`;
6286
const newDirPath = `${newApi}/${baseDir}`;
6387

64-
// Skip if not a directory or if matching directory doesn't exist in new
65-
if (
66-
!(await fs.stat(baseDirPath)).isDirectory() ||
67-
!newDirectories.includes(baseDir)
68-
) {
88+
// Skip if not a directory
89+
if (!(await fs.stat(baseDirPath)).isDirectory()) {
90+
continue;
91+
}
92+
93+
// Check if matching directory doesn't exist in new
94+
if (!newDirectories.includes(baseDir)) {
95+
console.log(`${baseDir} API is deleted`);
96+
allResults.push(`======${baseDir} API is deleted======`);
97+
hasChanges = true;
6998
continue;
7099
}
71100

72101
console.log(`Processing directory pair: ${baseDir}`);
73102

74103
try {
75-
const baseYamlPath = `"${baseDirPath}/**/*.yaml"`;
76-
const newYamlPath = `"${newDirPath}/**/*.yaml"`;
104+
const baseYamlPath = `${baseDirPath}/**/*.yaml`;
105+
const newYamlPath = `${newDirPath}/**/*.yaml`;
77106

78-
const oasdiffOutput = execSync(
79-
`oasdiff changelog ${jsonMode} ${directoryMode} ${baseYamlPath} ${newYamlPath}`
80-
).toString();
107+
const oasdiffOutput = await executeOasDiff(
108+
baseYamlPath,
109+
newYamlPath,
110+
jsonMode,
111+
directoryMode
112+
);
81113

82114
if (oasdiffOutput.trim().length > 0) {
83115
console.log(`Changes found in ${baseDir}`);
@@ -99,12 +131,26 @@ export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
99131
hasErrors = true;
100132
}
101133
}
134+
135+
// Check for newly added APIs (directories in new but not in base)
136+
for (const newDir of newDirectories) {
137+
const newDirPath = `${newApi}/${newDir}`;
138+
139+
// Skip if not a directory
140+
if (!(await fs.stat(newDirPath)).isDirectory()) {
141+
continue;
142+
}
143+
144+
// Check if this directory doesn't exist in base (added API)
145+
if (!baseDirectories.includes(newDir)) {
146+
console.log(`${newDir} API is added`);
147+
allResults.push(`======${newDir} API is added======`);
148+
hasChanges = true;
149+
}
150+
}
102151
} else {
103-
// Handle single file mode
104152
try {
105-
const oasdiffOutput = execSync(
106-
`oasdiff changelog ${jsonMode} "${baseApi}" "${newApi}"`
107-
).toString();
153+
const oasdiffOutput = await executeOasDiff(baseApi, newApi, jsonMode);
108154

109155
if (oasdiffOutput.trim().length > 0) {
110156
console.log("Changes found");
@@ -143,9 +189,9 @@ export async function oasDiffChangelog(baseApi: string, newApi: string, flags) {
143189
}
144190
}
145191

146-
export function checkOasDiffIsInstalled() {
192+
export async function checkOasDiffIsInstalled() {
147193
try {
148-
execSync(`oasdiff --version`).toString();
194+
await execAsync(`oasdiff --version`);
149195
} catch (err) {
150196
throw new Error(
151197
"oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"

0 commit comments

Comments
 (0)