Skip to content

Commit 1d249cf

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

File tree

2 files changed

+107
-76
lines changed

2 files changed

+107
-76
lines changed

‎src/diff/oasDiff.test.ts‎

Lines changed: 81 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ const pq = proxyquire.noCallThru();
1212

1313
describe("oasDiffChangelog", () => {
1414
it("should execute oasdiff command with correct parameters for single file mode", async () => {
15-
const execSyncStub = sinon.stub();
16-
execSyncStub.onCall(0).returns("version 1.0.0"); // version check
17-
execSyncStub.onCall(1).returns(""); // diff result
15+
const execStub = sinon.stub();
16+
// Mock the callback-style exec function
17+
execStub.callsArgWith(1, null, "", ""); // version check
18+
execStub.onSecondCall().callsArgWith(1, null, "", ""); // diff result
1819

1920
const fsStub = {
2021
readdir: sinon.stub().returns(["api-v1"]),
@@ -23,7 +24,7 @@ describe("oasDiffChangelog", () => {
2324

2425
const oasDiff = pq("./oasDiff", {
2526
child_process: {
26-
execSync: execSyncStub,
27+
exec: execStub,
2728
},
2829
"fs-extra": fsStub,
2930
});
@@ -34,17 +35,17 @@ describe("oasDiffChangelog", () => {
3435
const flags = {};
3536
const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
3637

37-
expect(execSyncStub.called).to.be.true;
38-
expect(execSyncStub.args[1][0]).to.equal(
39-
'oasdiff changelog "base.yaml" "new.yaml"'
38+
expect(execStub.called).to.be.true;
39+
expect(execStub.args[1][0]).to.equal(
40+
'oasdiff changelog "base.yaml" "new.yaml"'
4041
);
4142
expect(result).to.equal(0);
4243
});
4344

4445
it("should execute oasdiff command with correct parameters for directory mode", async () => {
45-
const execSyncStub = sinon.stub();
46-
execSyncStub.onCall(0).returns("version 1.0.0"); // version check
47-
execSyncStub.onCall(1).returns(""); // diff result
46+
const execStub = sinon.stub();
47+
execStub.callsArgWith(1, null, "version 1.0.0", ""); // version check
48+
execStub.onSecondCall().callsArgWith(1, null, "", ""); // diff result
4849

4950
const fsStub = {
5051
readdir: sinon.stub().returns(["api-v1"]),
@@ -53,7 +54,7 @@ describe("oasDiffChangelog", () => {
5354

5455
const oasDiff = pq("./oasDiff", {
5556
child_process: {
56-
execSync: execSyncStub,
57+
exec: execStub,
5758
},
5859
"fs-extra": fsStub,
5960
});
@@ -64,15 +65,15 @@ describe("oasDiffChangelog", () => {
6465
const flags = { dir: true };
6566
const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
6667

67-
expect(execSyncStub.called).to.be.true;
68-
expect(execSyncStub.args[1][0]).to.include('"base/api-v1/**/*.yaml"');
68+
expect(execStub.called).to.be.true;
69+
expect(execStub.args[1][0]).to.include("base/api-v1/**/*.yaml");
6970
expect(result).to.equal(0);
7071
});
7172

7273
it("should return 1 when oasdiff returns a non-empty string", async () => {
73-
const execSyncStub = sinon.stub();
74-
execSyncStub.onCall(0).returns("version 1.0.0");
75-
execSyncStub.onCall(1).returns("mock oasdiff change");
74+
const execStub = sinon.stub();
75+
execStub.callsArgWith(1, null, "version 1.0.0", "");
76+
execStub.onSecondCall().callsArgWith(1, null, "mock oasdiff change", "");
7677

7778
const fsStub = {
7879
readdir: sinon.stub().returns(["api-v1"]),
@@ -81,7 +82,7 @@ describe("oasDiffChangelog", () => {
8182

8283
const oasDiff = pq("./oasDiff", {
8384
child_process: {
84-
execSync: execSyncStub,
85+
exec: execStub,
8586
},
8687
"fs-extra": fsStub,
8788
});
@@ -91,14 +92,14 @@ describe("oasDiffChangelog", () => {
9192
const flags = {};
9293
const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
9394

94-
expect(execSyncStub.called).to.be.true;
95+
expect(execStub.called).to.be.true;
9596
expect(result).to.equal(1);
9697
});
9798

9899
it("should return 2 when oasdiff throws an error", async () => {
99-
const execSyncStub = sinon.stub();
100-
execSyncStub.onCall(0).returns("version 1.0.0");
101-
execSyncStub.onCall(1).throws(new Error("mock oasdiff error"));
100+
const execStub = sinon.stub();
101+
execStub.callsArgWith(1, null, "version 1.0.0", "");
102+
execStub.onSecondCall().callsArgWith(1, new Error("mock oasdiff error"));
102103

103104
const fsStub = {
104105
readdir: sinon.stub().returns(["api-v1"]),
@@ -107,7 +108,7 @@ describe("oasDiffChangelog", () => {
107108

108109
const oasDiff = pq("./oasDiff", {
109110
child_process: {
110-
execSync: execSyncStub,
111+
exec: execStub,
111112
},
112113
"fs-extra": fsStub,
113114
});
@@ -117,14 +118,14 @@ describe("oasDiffChangelog", () => {
117118
const flags = {};
118119
const result = await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
119120

120-
expect(execSyncStub.called).to.be.true;
121+
expect(execStub.called).to.be.true;
121122
expect(result).to.equal(2);
122123
});
123124

124125
it("should run oasdiff in directory mode when the --dir flag is provided", async () => {
125-
const execSyncStub = sinon.stub();
126-
execSyncStub.onCall(0).returns("version 1.0.0");
127-
execSyncStub.onCall(1).returns("a minor change");
126+
const execStub = sinon.stub();
127+
execStub.callsArgWith(1, null, "version 1.0.0", "");
128+
execStub.onSecondCall().callsArgWith(1, null, "a minor change", "");
128129

129130
const fsStub = {
130131
readdir: sinon.stub().returns(["api-v1"]),
@@ -133,7 +134,7 @@ describe("oasDiffChangelog", () => {
133134

134135
const oasDiff = pq("./oasDiff", {
135136
child_process: {
136-
execSync: execSyncStub,
137+
exec: execStub,
137138
},
138139
"fs-extra": fsStub,
139140
});
@@ -145,17 +146,17 @@ describe("oasDiffChangelog", () => {
145146
};
146147
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
147148

148-
expect(execSyncStub.called).to.be.true;
149-
expect(execSyncStub.args[1][0]).to.equal(
150-
'oasdiff changelog --composed "base/api-v1/**/*.yaml" "new/api-v1/**/*.yaml"'
149+
expect(execStub.called).to.be.true;
150+
expect(execStub.args[1][0]).to.equal(
151+
'oasdiff changelog --composed "base/api-v1/**/*.yaml" "new/api-v1/**/*.yaml"'
151152
);
152153
});
153154

154155
it("should concatenate results from multiple directories in text format", async () => {
155-
const execSyncStub = sinon.stub();
156-
execSyncStub.onCall(0).returns("version 1.0.0");
157-
execSyncStub.onCall(1).returns("changes in api-v1");
158-
execSyncStub.onCall(2).returns("changes in api-v2");
156+
const execStub = sinon.stub();
157+
execStub.callsArgWith(1, null, "version 1.0.0", "");
158+
execStub.onSecondCall().callsArgWith(1, null, "changes in api-v1", "");
159+
execStub.onThirdCall().callsArgWith(1, null, "changes in api-v2", "");
159160

160161
const fsStub = {
161162
readdir: sinon.stub().returns(["api-v1", "api-v2"]),
@@ -165,7 +166,7 @@ describe("oasDiffChangelog", () => {
165166

166167
const oasDiff = pq("./oasDiff", {
167168
child_process: {
168-
execSync: execSyncStub,
169+
exec: execStub,
169170
},
170171
"fs-extra": fsStub,
171172
});
@@ -188,10 +189,14 @@ describe("oasDiffChangelog", () => {
188189
});
189190

190191
it("should concatenate results from multiple directories in JSON format", async () => {
191-
const execSyncStub = sinon.stub();
192-
execSyncStub.onCall(0).returns("version 1.0.0");
193-
execSyncStub.onCall(1).returns('{"changes": "in api-v1"}');
194-
execSyncStub.onCall(2).returns('{"changes": "in api-v2"}');
192+
const execStub = sinon.stub();
193+
execStub.callsArgWith(1, null, "version 1.0.0", "");
194+
execStub
195+
.onSecondCall()
196+
.callsArgWith(1, null, '{"changes": "in api-v1"}', "");
197+
execStub
198+
.onThirdCall()
199+
.callsArgWith(1, null, '{"changes": "in api-v2"}', "");
195200

196201
const fsStub = {
197202
readdir: sinon.stub().returns(["api-v1", "api-v2"]),
@@ -201,7 +206,7 @@ describe("oasDiffChangelog", () => {
201206

202207
const oasDiff = pq("./oasDiff", {
203208
child_process: {
204-
execSync: execSyncStub,
209+
exec: execStub,
205210
},
206211
"fs-extra": fsStub,
207212
});
@@ -230,9 +235,9 @@ describe("oasDiffChangelog", () => {
230235
});
231236

232237
it("should skip non-directory entries", async () => {
233-
const execSyncStub = sinon.stub();
234-
execSyncStub.onCall(0).returns("version 1.0.0");
235-
execSyncStub.onCall(1).returns("changes in api-v1");
238+
const execStub = sinon.stub();
239+
execStub.callsArgWith(1, null, "version 1.0.0", "");
240+
execStub.onSecondCall().callsArgWith(1, null, "changes in api-v1", "");
236241

237242
const fsStub = {
238243
readdir: sinon.stub().returns(["api-v1", "not-a-dir.txt"]),
@@ -246,7 +251,7 @@ describe("oasDiffChangelog", () => {
246251

247252
const oasDiff = pq("./oasDiff", {
248253
child_process: {
249-
execSync: execSyncStub,
254+
exec: execStub,
250255
},
251256
"fs-extra": fsStub,
252257
});
@@ -257,21 +262,21 @@ describe("oasDiffChangelog", () => {
257262

258263
await oasDiff.oasDiffChangelog(baseApi, newApi, flags);
259264

260-
// Should only call execSync twice (once for version check, once for api-v1)
261-
expect(execSyncStub.callCount).to.equal(2);
265+
// Should only call exec twice (once for version check, once for api-v1)
266+
expect(execStub.callCount).to.equal(2);
262267
});
263268

264269
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");
270+
const execStub = sinon.stub();
271+
execStub.callsArgWith(1, null, "version 1.0.0", "");
267272

268273
const fsStub = {
269274
readdir: sinon.stub(),
270275
stat: sinon.stub(),
271276
writeFile: sinon.stub(),
272277
};
273278

274-
// Base has api-v1 and api-v2, new only has api-v1
279+
// Base has api-v1 and api-v2, new only has api-v2
275280
fsStub.readdir.onCall(0).returns(["api-v1", "api-v2"]); // base directories
276281
fsStub.readdir.onCall(1).returns(["api-v2"]); // new directories
277282

@@ -280,7 +285,7 @@ describe("oasDiffChangelog", () => {
280285

281286
const oasDiff = pq("./oasDiff", {
282287
child_process: {
283-
execSync: execSyncStub,
288+
exec: execStub,
284289
},
285290
"fs-extra": fsStub,
286291
});
@@ -300,8 +305,8 @@ describe("oasDiffChangelog", () => {
300305
});
301306

302307
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");
308+
const execStub = sinon.stub();
309+
execStub.callsArgWith(1, null, "version 1.0.0", "");
305310

306311
const fsStub = {
307312
readdir: sinon.stub(),
@@ -318,7 +323,7 @@ describe("oasDiffChangelog", () => {
318323

319324
const oasDiff = pq("./oasDiff", {
320325
child_process: {
321-
execSync: execSyncStub,
326+
exec: execStub,
322327
},
323328
"fs-extra": fsStub,
324329
});
@@ -338,17 +343,17 @@ describe("oasDiffChangelog", () => {
338343
});
339344

340345
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");
346+
const execStub = sinon.stub();
347+
execStub.callsArgWith(1, null, "version 1.0.0", "");
348+
execStub.onSecondCall().callsArgWith(1, null, "changes in api-v1", "");
344349

345350
const fsStub = {
346351
readdir: sinon.stub(),
347352
stat: sinon.stub(),
348353
writeFile: sinon.stub(),
349354
};
350355

351-
// Base has api-v1 and api-v2, new has api-v1 and api-v3
356+
// Base has api-v1 and api-v2, new has api-v2 and api-v3
352357
fsStub.readdir.onCall(0).returns(["api-v1", "api-v2"]); // base directories
353358
fsStub.readdir.onCall(1).returns(["api-v2", "api-v3"]); // new directories
354359

@@ -357,7 +362,7 @@ describe("oasDiffChangelog", () => {
357362

358363
const oasDiff = pq("./oasDiff", {
359364
child_process: {
360-
execSync: execSyncStub,
365+
exec: execStub,
361366
},
362367
"fs-extra": fsStub,
363368
});
@@ -379,10 +384,10 @@ describe("oasDiffChangelog", () => {
379384
});
380385

381386
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
387+
const execStub = sinon.stub();
388+
execStub.callsArgWith(1, null, "version 1.0.0", "");
389+
execStub.onSecondCall().callsArgWith(1, null, "changes in common-api", "");
390+
execStub.onThirdCall().callsArgWith(1, null, "", ""); // no changes in stable-api
386391

387392
const fsStub = {
388393
readdir: sinon.stub(),
@@ -400,7 +405,7 @@ describe("oasDiffChangelog", () => {
400405

401406
const oasDiff = pq("./oasDiff", {
402407
child_process: {
403-
execSync: execSyncStub,
408+
exec: execStub,
404409
},
405410
"fs-extra": fsStub,
406411
});
@@ -431,15 +436,23 @@ describe("oasDiffChangelog", () => {
431436
expect(writtenContent).to.not.include("=== Changes in stable-api ===");
432437
});
433438

434-
it("should throw an error if oasdiff is not installed", () => {
439+
it("should throw an error if oasdiff is not installed", async () => {
440+
const execStub = sinon.stub();
441+
execStub.callsArgWith(1, new Error("oasdiff not installed"));
442+
435443
const oasDiff = pq("./oasDiff", {
436444
child_process: {
437-
execSync: sinon.stub().throws(new Error("oasdiff not installed")),
445+
exec: execStub,
438446
},
439447
});
440448

441-
expect(() => oasDiff.checkOasDiffIsInstalled()).to.throw(
442-
"oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"
443-
);
449+
try {
450+
await oasDiff.checkOasDiffIsInstalled();
451+
expect.fail("Expected function to throw an error");
452+
} catch (error) {
453+
expect(error.message).to.equal(
454+
"oasdiff is not installed. Install oasdiff according to https://github.com/oasdiff/oasdiff#installation"
455+
);
456+
}
444457
});
445458
});

0 commit comments

Comments
 (0)