Skip to content

Commit c5e26c1

Browse files
kmaschikmaschi
authored
Merge pull request #42 from adobe/chore/sourcehooks-remote-fix
Chore/sourcehooks remote fix
2 parents bbf8498 + 37f9fd3 commit c5e26c1

14 files changed

+490
-369
lines changed

‎src/__mocks__/setResultAndStopExecution.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/*
2+
Copyright 2022 Adobe. All rights reserved.
3+
This file is licensed to you under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License. You may obtain a copy
5+
of the License at http://www.apache.org/licenses/LICENSE-2.0
6+
7+
Unless required by applicable law or agreed to in writing, software distributed under
8+
the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
9+
OF ANY KIND, either express or implied. See the License for the specific language
10+
governing permissions and limitations under the License.
11+
*/
12+
13+
import { vi } from 'vitest';
14+
15+
const mockSetResultAndStopExecution = vi.fn();
16+
17+
export { mockSetResultAndStopExecution };

‎src/__tests__/handleAfterAllHooks.test.ts

Lines changed: 95 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,15 @@ OF ANY KIND, either express or implied. See the License for the specific languag
1010
governing permissions and limitations under the License.
1111
*/
1212

13+
import { mockSetResultAndStopExecution } from '../__mocks__/setResultAndStopExecution';
1314
import getAfterAllHookHandler, { AfterAllHookBuildConfig } from '../handleAfterAllHooks';
14-
import { HookResponse, HookStatus, GraphQLResult, HookFunctionPayloadContext } from '../types';
15+
import {
16+
HookResponse,
17+
HookStatus,
18+
GraphQLResult,
19+
HookFunctionPayloadContext,
20+
AfterAllHookResponse,
21+
} from '../types';
1522
import { mockLogger } from '../__mocks__/yogaLogger';
1623
import { describe, expect, test, vi, beforeEach } from 'vitest';
1724

@@ -60,9 +67,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
6067
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
6168
};
6269
const handler = getAfterAllHookHandler(mockConfig);
63-
const result = await handler({ payload: basePayload });
64-
expect(result).toEqual(mockResponse);
70+
await handler({
71+
payload: basePayload,
72+
setResultAndStopExecution: mockSetResultAndStopExecution,
73+
});
6574
expect(mockHook).toHaveBeenCalledOnce();
75+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
76+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
6677
});
6778

6879
test('throws if blocking and status is ERROR', async () => {
@@ -78,7 +89,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
7889
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
7990
};
8091
const handler = getAfterAllHookHandler(mockConfig);
81-
await expect(handler({ payload: basePayload })).rejects.toThrow('fail');
92+
await expect(
93+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
94+
).rejects.toThrow('fail');
8295
});
8396

8497
test('does not throw if non-blocking and status is ERROR', async () => {
@@ -94,15 +107,19 @@ describe('getAfterAllHookHandler (afterAll)', () => {
94107
afterAll: { blocking: false, module: { mockHook }, fn: 'mockHook' },
95108
};
96109
const handler = getAfterAllHookHandler(mockConfig);
97-
const result = await handler({ payload: basePayload });
98-
expect(result).toEqual(mockResponse);
110+
await handler({
111+
payload: basePayload,
112+
setResultAndStopExecution: mockSetResultAndStopExecution,
113+
});
99114
expect(mockHook).toHaveBeenCalledOnce();
115+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
116+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
100117
});
101118

102119
test('returns modified result if present in response', async () => {
103120
vi.mocked(utils.isModuleFn).mockReturnValue(true);
104-
const modifiedResult = { data: { foo: 'bar' }, errors: [] };
105-
const mockResponse: HookResponse = {
121+
const modifiedResult = { data: { foo: 'bar' }, errors: [], extensions: {} };
122+
const mockResponse: AfterAllHookResponse = {
106123
status: HookStatus.SUCCESS,
107124
message: 'modified',
108125
data: { result: modifiedResult },
@@ -117,31 +134,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
117134
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
118135
};
119136
const handler = getAfterAllHookHandler(mockConfig);
120-
const result = await handler({ payload: basePayload });
121-
expect(result).toEqual(mockResponse);
122-
expect(mockHook).toHaveBeenCalledOnce();
123-
});
124-
125-
test('handles headers in response data', async () => {
126-
vi.mocked(utils.isModuleFn).mockReturnValue(true);
127-
const mockResponse: HookResponse = {
128-
status: HookStatus.SUCCESS,
129-
message: 'headers',
130-
data: { headers: { 'x-test': 'abc' } },
131-
};
132-
const mockHook = vi.fn().mockResolvedValue(mockResponse);
133-
vi.mocked(utils.getWrappedLocalModuleHookFunction).mockResolvedValue(mockHook);
134-
vi.mocked(utils.getWrappedLocalHookFunction).mockResolvedValue(mockHook);
135-
const mockConfig: AfterAllHookBuildConfig = {
136-
memoizedFns: {},
137-
baseDir: '',
138-
logger: mockLogger,
139-
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
140-
};
141-
const handler = getAfterAllHookHandler(mockConfig);
142-
const result = await handler({ payload: basePayload });
143-
expect(result).toEqual(mockResponse);
137+
await handler({
138+
payload: basePayload,
139+
setResultAndStopExecution: mockSetResultAndStopExecution,
140+
});
144141
expect(mockHook).toHaveBeenCalledOnce();
142+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
143+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(modifiedResult);
145144
});
146145

147146
test('throws if hook throws (blocking)', async () => {
@@ -158,7 +157,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
158157
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
159158
};
160159
const handler = getAfterAllHookHandler(mockConfig);
161-
await expect(handler({ payload: basePayload })).rejects.toThrow('fail!');
160+
await expect(
161+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
162+
).rejects.toThrow('fail!');
162163
});
163164

164165
test('throws error if no hook is defined', async () => {
@@ -174,9 +175,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
174175
afterAll: { blocking: true },
175176
};
176177
const handler = getAfterAllHookHandler(mockConfig);
177-
await expect(handler({ payload: basePayload })).rejects.toThrow(
178-
'Unable to invoke local function undefined',
179-
);
178+
await expect(
179+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
180+
).rejects.toThrow('Unable to invoke local function undefined');
180181
});
181182

182183
test('uses memoized function if present', async () => {
@@ -190,9 +191,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
190191
afterAll: { blocking: true },
191192
};
192193
const handler = getAfterAllHookHandler(mockConfig);
193-
const result = await handler({ payload: basePayload });
194-
expect(result).toEqual(mockResponse);
194+
await handler({
195+
payload: basePayload,
196+
setResultAndStopExecution: mockSetResultAndStopExecution,
197+
});
195198
expect(memoized).toHaveBeenCalledOnce();
199+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
200+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
196201
});
197202

198203
test('handles invalid response (missing status) for blocking', async () => {
@@ -207,7 +212,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
207212
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
208213
};
209214
const handler = getAfterAllHookHandler(mockConfig);
210-
await expect(handler({ payload: basePayload })).rejects.toThrow();
215+
await expect(
216+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
217+
).rejects.toThrow();
211218
});
212219

213220
test('handles remote hook (success)', async () => {
@@ -223,9 +230,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
223230
afterAll: { blocking: true, composer: 'https://remote' },
224231
};
225232
const handler = getAfterAllHookHandler(mockConfig);
226-
const result = await handler({ payload: basePayload });
227-
expect(result).toEqual({ status: HookStatus.SUCCESS, message: 'remote ok' });
233+
await handler({
234+
payload: basePayload,
235+
setResultAndStopExecution: mockSetResultAndStopExecution,
236+
});
228237
expect(mockRemoteHook).toHaveBeenCalledOnce();
238+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
239+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
229240
});
230241

231242
test('handles remote hook (error, blocking)', async () => {
@@ -241,7 +252,12 @@ describe('getAfterAllHookHandler (afterAll)', () => {
241252
afterAll: { blocking: true, composer: 'https://remote' },
242253
};
243254
const handler = getAfterAllHookHandler(mockConfig);
244-
await expect(handler({ payload: basePayload })).rejects.toThrow('remote fail');
255+
await expect(
256+
handler({
257+
payload: basePayload,
258+
setResultAndStopExecution: mockSetResultAndStopExecution,
259+
}),
260+
).rejects.toThrow('remote fail');
245261
});
246262

247263
test('handles remote hook (error, non-blocking)', async () => {
@@ -257,9 +273,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
257273
afterAll: { blocking: false, composer: 'https://remote' },
258274
};
259275
const handler = getAfterAllHookHandler(mockConfig);
260-
const result = await handler({ payload: basePayload });
261-
expect(result).toEqual({ status: HookStatus.ERROR, message: 'remote fail' });
276+
await handler({
277+
payload: basePayload,
278+
setResultAndStopExecution: mockSetResultAndStopExecution,
279+
});
262280
expect(mockRemoteHook).toHaveBeenCalledOnce();
281+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
282+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
263283
});
264284

265285
test('handles case-insensitive status comparison', async () => {
@@ -275,9 +295,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
275295
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
276296
};
277297
const handler = getAfterAllHookHandler(mockConfig);
278-
const result = await handler({ payload: basePayload });
279-
expect(result).toEqual(mockResponse);
298+
await handler({
299+
payload: basePayload,
300+
setResultAndStopExecution: mockSetResultAndStopExecution,
301+
});
280302
expect(mockHook).toHaveBeenCalledOnce();
303+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
304+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
281305
});
282306

283307
test('handles error with non-Error object', async () => {
@@ -294,7 +318,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
294318
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
295319
};
296320
const handler = getAfterAllHookHandler(mockConfig);
297-
await expect(handler({ payload: basePayload })).rejects.toThrow('custom error object');
321+
await expect(
322+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
323+
).rejects.toThrow('custom error object');
298324
});
299325

300326
test('handles error without message property', async () => {
@@ -311,9 +337,9 @@ describe('getAfterAllHookHandler (afterAll)', () => {
311337
afterAll: { blocking: true, module: { mockHook }, fn: 'mockHook' },
312338
};
313339
const handler = getAfterAllHookHandler(mockConfig);
314-
await expect(handler({ payload: basePayload })).rejects.toThrow(
315-
'Error while invoking afterAll hook',
316-
);
340+
await expect(
341+
handler({ payload: basePayload, setResultAndStopExecution: mockSetResultAndStopExecution }),
342+
).rejects.toThrow('Error while invoking afterAll hook');
317343
});
318344

319345
test('handles local function with composer path', async () => {
@@ -329,9 +355,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
329355
afterAll: { blocking: true, composer: './local-hook.js' },
330356
};
331357
const handler = getAfterAllHookHandler(mockConfig);
332-
const result = await handler({ payload: basePayload });
333-
expect(result).toEqual(mockResponse);
358+
await handler({
359+
payload: basePayload,
360+
setResultAndStopExecution: mockSetResultAndStopExecution,
361+
});
334362
expect(mockHook).toHaveBeenCalledOnce();
363+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
364+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith(basePayload.result);
335365
});
336366

337367
test('handles payload with null result', async () => {
@@ -352,9 +382,13 @@ describe('getAfterAllHookHandler (afterAll)', () => {
352382
document: {},
353383
result: null as unknown as GraphQLResult,
354384
};
355-
const result = await handler({ payload: payloadWithNullResult });
356-
expect(result).toEqual(mockResponse);
385+
await handler({
386+
payload: payloadWithNullResult,
387+
setResultAndStopExecution: mockSetResultAndStopExecution,
388+
});
357389
expect(mockHook).toHaveBeenCalledWith(payloadWithNullResult);
390+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
391+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith({});
358392
});
359393

360394
test('handles payload with undefined result', async () => {
@@ -375,8 +409,12 @@ describe('getAfterAllHookHandler (afterAll)', () => {
375409
document: {},
376410
result: undefined,
377411
};
378-
const result = await handler({ payload: payloadWithUndefinedResult });
379-
expect(result).toEqual(mockResponse);
380-
expect(mockHook).toHaveBeenCalledWith(payloadWithUndefinedResult);
412+
await handler({
413+
payload: payloadWithUndefinedResult,
414+
setResultAndStopExecution: mockSetResultAndStopExecution,
415+
});
416+
expect(mockHook).toHaveBeenCalledOnce();
417+
expect(mockSetResultAndStopExecution).toHaveBeenCalledOnce();
418+
expect(mockSetResultAndStopExecution).toHaveBeenCalledWith({});
381419
});
382420
});

0 commit comments

Comments
 (0)