Skip to content

Commit 61dfabf

Browse files
SakshiKoli-CSSakshiKoli-CS
authored
Merge pull request #73 from contentstack/cl-2125
fix: handle empty server command input properly
2 parents 5af1aac + 93a1222 commit 61dfabf

File tree

5 files changed

+628
-10
lines changed

5 files changed

+628
-10
lines changed

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,6 @@
55
- For the happy path, can we have a single unit test where all the top level if conditions are executed? This might help with reducing the number of total unit tests created and still give same test coverage.
66
- For the tests for edge cases do not create separate describe blocks, keep the hierarchy flat.
77
- For the tests for edge cases, do not skip assertions, its still worth adding all assertions similar to the happy paths tests.
8+
- Use only jest for writing test cases and refer existing unit test under the /src folder.
9+
- Do not create code comments for any changes.
810

‎src/adapters/file-upload.test.ts‎

Lines changed: 319 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
import FileUpload from './file-upload';
2+
import BaseClass from './base-class';
3+
import { cliux } from '@contentstack/cli-utilities';
4+
import { DeploymentStatus } from '../types/launch';
5+
6+
jest.mock('@contentstack/cli-utilities', () => ({
7+
...jest.requireActual('@contentstack/cli-utilities'),
8+
cliux: {
9+
inquire: jest.fn(),
10+
loader: jest.fn(),
11+
print: jest.fn(),
12+
},
13+
configHandler: {
14+
get: jest.fn(),
15+
},
16+
HttpClient: jest.fn(),
17+
}));
18+
19+
describe('FileUpload Adapter', () => {
20+
let logMock: jest.Mock;
21+
let exitMock: jest.Mock;
22+
23+
beforeEach(() => {
24+
logMock = jest.fn();
25+
exitMock = jest.fn().mockImplementation(() => {
26+
throw new Error('1');
27+
});
28+
});
29+
30+
afterEach(() => {
31+
jest.resetAllMocks();
32+
});
33+
34+
describe('runFileUploadFlow', () => {
35+
let getEnvironmentMock: jest.SpyInstance;
36+
let handleExistingProjectMock: jest.SpyInstance;
37+
let handleNewProjectMock: jest.SpyInstance;
38+
let prepareLaunchConfigMock: jest.SpyInstance;
39+
let showLogsMock: jest.SpyInstance;
40+
let showDeploymentUrlMock: jest.SpyInstance;
41+
let showSuggestionMock: jest.SpyInstance;
42+
43+
beforeEach(() => {
44+
getEnvironmentMock = jest
45+
.spyOn(BaseClass.prototype, 'getEnvironment')
46+
.mockResolvedValue({ uid: 'env-123', name: 'Default', frameworkPreset: 'OTHER' });
47+
handleExistingProjectMock = jest
48+
.spyOn(FileUpload.prototype as any, 'handleExistingProject')
49+
.mockResolvedValue(undefined);
50+
handleNewProjectMock = jest
51+
.spyOn(FileUpload.prototype as any, 'handleNewProject')
52+
.mockResolvedValue(undefined);
53+
prepareLaunchConfigMock = jest.spyOn(BaseClass.prototype, 'prepareLaunchConfig').mockImplementation(() => {});
54+
showLogsMock = jest.spyOn(BaseClass.prototype, 'showLogs').mockResolvedValue(true);
55+
showDeploymentUrlMock = jest.spyOn(BaseClass.prototype, 'showDeploymentUrl').mockImplementation(() => {});
56+
showSuggestionMock = jest.spyOn(BaseClass.prototype, 'showSuggestion').mockImplementation(() => {});
57+
});
58+
59+
afterEach(() => {
60+
getEnvironmentMock.mockRestore();
61+
handleExistingProjectMock.mockRestore();
62+
handleNewProjectMock.mockRestore();
63+
prepareLaunchConfigMock.mockRestore();
64+
showLogsMock.mockRestore();
65+
showDeploymentUrlMock.mockRestore();
66+
showSuggestionMock.mockRestore();
67+
});
68+
69+
it('should exit with code 1 when deployment status is FAILED for existing project', async () => {
70+
const fileUploadInstance = new FileUpload({
71+
config: {
72+
isExistingProject: true,
73+
currentDeploymentStatus: DeploymentStatus.FAILED,
74+
},
75+
log: logMock,
76+
exit: exitMock,
77+
} as any);
78+
79+
try {
80+
await fileUploadInstance.run();
81+
} catch (error: any) {
82+
expect(error.message).toBe('1');
83+
}
84+
85+
expect(getEnvironmentMock).toHaveBeenCalled();
86+
expect(handleExistingProjectMock).toHaveBeenCalled();
87+
expect(prepareLaunchConfigMock).toHaveBeenCalled();
88+
expect(showLogsMock).toHaveBeenCalled();
89+
expect(exitMock).toHaveBeenCalledWith(1);
90+
expect(showDeploymentUrlMock).not.toHaveBeenCalled();
91+
expect(showSuggestionMock).not.toHaveBeenCalled();
92+
});
93+
94+
it('should exit with code 1 when deployment status is FAILED for new project', async () => {
95+
const fileUploadInstance = new FileUpload({
96+
config: {
97+
isExistingProject: false,
98+
currentDeploymentStatus: DeploymentStatus.FAILED,
99+
},
100+
log: logMock,
101+
exit: exitMock,
102+
} as any);
103+
104+
try {
105+
await fileUploadInstance.run();
106+
} catch (error: any) {
107+
expect(error.message).toBe('1');
108+
}
109+
110+
expect(handleNewProjectMock).toHaveBeenCalled();
111+
expect(prepareLaunchConfigMock).toHaveBeenCalled();
112+
expect(showLogsMock).toHaveBeenCalled();
113+
expect(exitMock).toHaveBeenCalledWith(1);
114+
expect(showDeploymentUrlMock).not.toHaveBeenCalled();
115+
expect(showSuggestionMock).not.toHaveBeenCalled();
116+
});
117+
118+
it('should continue normally when deployment status is not FAILED', async () => {
119+
const fileUploadInstance = new FileUpload({
120+
config: {
121+
isExistingProject: true,
122+
currentDeploymentStatus: DeploymentStatus.LIVE,
123+
},
124+
log: logMock,
125+
exit: exitMock,
126+
} as any);
127+
128+
await fileUploadInstance.run();
129+
130+
expect(getEnvironmentMock).toHaveBeenCalled();
131+
expect(handleExistingProjectMock).toHaveBeenCalled();
132+
expect(prepareLaunchConfigMock).toHaveBeenCalled();
133+
expect(showLogsMock).toHaveBeenCalled();
134+
expect(exitMock).not.toHaveBeenCalled();
135+
expect(showDeploymentUrlMock).toHaveBeenCalled();
136+
expect(showSuggestionMock).toHaveBeenCalled();
137+
});
138+
});
139+
140+
describe('prepareAndUploadNewProjectFile', () => {
141+
let selectOrgMock: jest.SpyInstance;
142+
let createSignedUploadUrlMock: jest.SpyInstance;
143+
let archiveMock: jest.SpyInstance;
144+
let uploadFileMock: jest.SpyInstance;
145+
let detectFrameworkMock: jest.SpyInstance;
146+
let handleEnvImportFlowMock: jest.SpyInstance;
147+
148+
beforeEach(() => {
149+
selectOrgMock = jest.spyOn(BaseClass.prototype, 'selectOrg').mockResolvedValue();
150+
createSignedUploadUrlMock = jest.spyOn(FileUpload.prototype, 'createSignedUploadUrl').mockResolvedValue({
151+
uploadUid: 'upload-123',
152+
uploadUrl: 'https://example.com/upload',
153+
fields: [],
154+
headers: [],
155+
method: 'PUT',
156+
});
157+
archiveMock = jest.spyOn(FileUpload.prototype, 'archive').mockResolvedValue({
158+
zipName: 'test.zip',
159+
zipPath: '/path/to/test.zip',
160+
projectName: 'test-project',
161+
});
162+
uploadFileMock = jest.spyOn(FileUpload.prototype, 'uploadFile').mockResolvedValue();
163+
detectFrameworkMock = jest.spyOn(BaseClass.prototype, 'detectFramework').mockResolvedValue();
164+
handleEnvImportFlowMock = jest.spyOn(BaseClass.prototype, 'handleEnvImportFlow').mockResolvedValue();
165+
});
166+
167+
afterEach(() => {
168+
selectOrgMock.mockRestore();
169+
createSignedUploadUrlMock.mockRestore();
170+
archiveMock.mockRestore();
171+
uploadFileMock.mockRestore();
172+
detectFrameworkMock.mockRestore();
173+
handleEnvImportFlowMock.mockRestore();
174+
});
175+
176+
it('should prompt for server command when framework supports it and serverCommand is not provided', async () => {
177+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('test-project'); // projectName
178+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('Default'); // environmentName
179+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm run build'); // buildCommand
180+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('./dist'); // outputDirectory
181+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm start'); // serverCommand
182+
183+
const fileUploadInstance = new FileUpload({
184+
config: {
185+
flags: {
186+
'server-command': undefined,
187+
},
188+
framework: 'OTHER',
189+
supportedFrameworksForServerCommands: ['ANGULAR', 'OTHER', 'REMIX', 'NUXT'],
190+
outputDirectories: { OTHER: './dist' },
191+
},
192+
log: logMock,
193+
exit: exitMock,
194+
} as any);
195+
196+
await fileUploadInstance.prepareAndUploadNewProjectFile();
197+
198+
expect(cliux.inquire).toHaveBeenCalledWith({
199+
type: 'input',
200+
name: 'serverCommand',
201+
message: 'Server Command',
202+
});
203+
expect(fileUploadInstance.config.serverCommand).toBe('npm start');
204+
});
205+
206+
it('should not prompt for server command when framework does not support it', async () => {
207+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('test-project');
208+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('Default');
209+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm run build');
210+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('./dist');
211+
212+
const fileUploadInstance = new FileUpload({
213+
config: {
214+
flags: {
215+
'server-command': undefined,
216+
},
217+
framework: 'GATSBY',
218+
supportedFrameworksForServerCommands: ['ANGULAR', 'OTHER', 'REMIX', 'NUXT'],
219+
outputDirectories: { GATSBY: './public' },
220+
},
221+
log: logMock,
222+
exit: exitMock,
223+
} as any);
224+
225+
await fileUploadInstance.prepareAndUploadNewProjectFile();
226+
227+
const serverCommandCalls = (cliux.inquire as jest.Mock).mock.calls.filter(
228+
(call) => call[0]?.name === 'serverCommand',
229+
);
230+
expect(serverCommandCalls.length).toBe(0);
231+
});
232+
233+
it('should not prompt for server command when serverCommand is already provided in flags', async () => {
234+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('test-project');
235+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('Default');
236+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm run build');
237+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('./dist');
238+
239+
const fileUploadInstance = new FileUpload({
240+
config: {
241+
flags: {
242+
'server-command': 'npm run serve',
243+
},
244+
framework: 'OTHER',
245+
supportedFrameworksForServerCommands: ['ANGULAR', 'OTHER', 'REMIX', 'NUXT'],
246+
outputDirectories: { OTHER: './dist' },
247+
},
248+
log: logMock,
249+
exit: exitMock,
250+
} as any);
251+
252+
await fileUploadInstance.prepareAndUploadNewProjectFile();
253+
254+
const serverCommandCalls = (cliux.inquire as jest.Mock).mock.calls.filter(
255+
(call) => call[0]?.name === 'serverCommand',
256+
);
257+
expect(serverCommandCalls.length).toBe(0);
258+
expect(fileUploadInstance.config.flags['server-command']).toBe('npm run serve');
259+
});
260+
261+
it('should not set serverCommand when user provides empty input', async () => {
262+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('test-project');
263+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('Default');
264+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm run build');
265+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('./dist');
266+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('');
267+
268+
const fileUploadInstance = new FileUpload({
269+
config: {
270+
flags: {
271+
'server-command': undefined,
272+
},
273+
framework: 'OTHER',
274+
supportedFrameworksForServerCommands: ['ANGULAR', 'OTHER', 'REMIX', 'NUXT'],
275+
outputDirectories: { OTHER: './dist' },
276+
},
277+
log: logMock,
278+
exit: exitMock,
279+
} as any);
280+
281+
await fileUploadInstance.prepareAndUploadNewProjectFile();
282+
283+
expect(cliux.inquire).toHaveBeenCalledWith({
284+
type: 'input',
285+
name: 'serverCommand',
286+
message: 'Server Command',
287+
});
288+
expect(fileUploadInstance.config.serverCommand).toBeUndefined();
289+
});
290+
291+
it('should not prompt for server command when framework is not set', async () => {
292+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('test-project');
293+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('Default');
294+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('npm run build');
295+
(cliux.inquire as jest.Mock).mockResolvedValueOnce('./dist');
296+
297+
const fileUploadInstance = new FileUpload({
298+
config: {
299+
flags: {
300+
'server-command': undefined,
301+
},
302+
framework: undefined,
303+
supportedFrameworksForServerCommands: ['ANGULAR', 'OTHER', 'REMIX', 'NUXT'],
304+
outputDirectories: { OTHER: './dist' },
305+
},
306+
log: logMock,
307+
exit: exitMock,
308+
} as any);
309+
310+
await fileUploadInstance.prepareAndUploadNewProjectFile();
311+
312+
const serverCommandCalls = (cliux.inquire as jest.Mock).mock.calls.filter(
313+
(call) => call[0]?.name === 'serverCommand',
314+
);
315+
expect(serverCommandCalls.length).toBe(0);
316+
});
317+
});
318+
});
319+

‎src/adapters/file-upload.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export default class FileUpload extends BaseClass {
129129
name: environmentName || 'Default',
130130
environmentVariables: map(this.envVariables, ({ key, value }) => ({ key, value })),
131131
buildCommand: buildCommand === undefined || buildCommand === null ? 'npm run build' : buildCommand,
132-
serverCommand: serverCommand === undefined || serverCommand === null ? 'npm run start' : serverCommand,
132+
...(serverCommand && serverCommand.trim() !== '' ? { serverCommand } : {}),
133133
},
134134
},
135135
skipGitData: true,
@@ -226,13 +226,18 @@ export default class FileUpload extends BaseClass {
226226
default: (this.config.outputDirectories as Record<string, string>)[this.config?.framework || 'OTHER'],
227227
}));
228228
if (this.config.framework && this.config.supportedFrameworksForServerCommands.includes(this.config.framework)) {
229-
this.config.serverCommand =
230-
serverCommand ||
231-
(await cliux.inquire({
229+
if (!serverCommand) {
230+
const serverCommandInput = await cliux.inquire({
232231
type: 'input',
233232
name: 'serverCommand',
234233
message: 'Server Command',
235-
}));
234+
});
235+
if (serverCommandInput) {
236+
this.config.serverCommand = serverCommandInput;
237+
}
238+
} else {
239+
this.config.serverCommand = serverCommand;
240+
}
236241
}
237242
this.config.variableType = variableType as unknown as string;
238243
this.config.envVariables = envVariables;

0 commit comments

Comments
 (0)