Skip to content

Commit f5d4d26

Browse files
SakshiKoli-CSSakshiKoli-CS
authored
Merge pull request #74 from contentstack/development
Fix issue regarding skipping env variables, suppressing non-dev error messages, parsing URL-formatted env values and handling empty server command inputs.
2 parents 931be6a + 61dfabf commit f5d4d26

File tree

15 files changed

+1157
-71
lines changed

15 files changed

+1157
-71
lines changed

‎.github/CODEOWNERS‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,9 @@
1-
* @contentstack/security-admin
1+
* @contentstack/launch-pr-reviewers
2+
3+
.github/workflows/sca-scan.yml @contentstack/security-admin
4+
5+
**/.snyk @contentstack/security-admin
6+
7+
.github/workflows/policy-scan.yml @contentstack/security-admin
8+
9+
.github/workflows/issues-jira.yml @contentstack/security-admin

‎.github/workflows/secrets-scan.yml‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

‎AGENTS.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
## Testing instructions
2+
3+
- Follow the Arrange, Act, Assert structure when writing unit tests.
4+
- When writing unit tests, create individual unit tests that cover each logical branching in the code.
5+
- 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.
6+
- For the tests for edge cases do not create separate describe blocks, keep the hierarchy flat.
7+
- 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.
10+

‎package-lock.json‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"/oclif.manifest.json"
2121
],
2222
"dependencies": {
23-
"@apollo/client": "^3.11.8",
23+
"@apollo/client": "^3.14.0",
2424
"@contentstack/cli-command": "^1.4.0",
2525
"@contentstack/cli-utilities": "^1.12.0",
2626
"@oclif/core": "^4.2.7",

‎src/adapters/base-class.test.ts‎

Lines changed: 252 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,213 @@ describe('BaseClass', () => {
3232
config: config.variablePreparationTypeOptions,
3333
} as any);
3434
});
35+
36+
it('should handle string variableType by converting to array - Import variables from a stack', async () => {
37+
baseClass = new BaseClass({
38+
log: logMock,
39+
exit: exitMock,
40+
config: {
41+
variableType: 'Import variables from a stack',
42+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
43+
},
44+
} as any);
45+
46+
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce();
47+
48+
await baseClass.handleEnvImportFlow();
49+
50+
expect(importEnvFromStackMock).toHaveBeenCalled();
51+
expect(exitMock).not.toHaveBeenCalled();
52+
});
53+
54+
it('should handle string variableType by converting to array - Manually add custom variables to the list', async () => {
55+
baseClass = new BaseClass({
56+
log: logMock,
57+
exit: exitMock,
58+
config: {
59+
variableType: 'Manually add custom variables to the list',
60+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
61+
},
62+
} as any);
63+
64+
const promptForEnvValuesMock = jest.spyOn(baseClass, 'promptForEnvValues').mockResolvedValueOnce();
65+
66+
await baseClass.handleEnvImportFlow();
67+
68+
expect(promptForEnvValuesMock).toHaveBeenCalled();
69+
expect(exitMock).not.toHaveBeenCalled();
70+
});
71+
72+
it('should handle string variableType by converting to array - Import variables from the .env.local file', async () => {
73+
baseClass = new BaseClass({
74+
log: logMock,
75+
exit: exitMock,
76+
config: {
77+
variableType: 'Import variables from the .env.local file',
78+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
79+
},
80+
} as any);
81+
82+
const importVariableFromLocalConfigMock = jest
83+
.spyOn(baseClass, 'importVariableFromLocalConfig')
84+
.mockResolvedValueOnce();
85+
86+
await baseClass.handleEnvImportFlow();
87+
88+
expect(importVariableFromLocalConfigMock).toHaveBeenCalled();
89+
expect(exitMock).not.toHaveBeenCalled();
90+
});
91+
92+
it('should handle string variableType by converting to array - Skip adding environment variables', async () => {
93+
baseClass = new BaseClass({
94+
log: logMock,
95+
exit: exitMock,
96+
config: {
97+
variableType: 'Skip adding environment variables',
98+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
99+
},
100+
} as any);
101+
102+
await baseClass.handleEnvImportFlow();
103+
104+
expect(baseClass.envVariables).toEqual([]);
105+
expect(logMock).toHaveBeenCalledWith('Skipped adding environment variables.', 'info');
106+
expect(exitMock).not.toHaveBeenCalled();
107+
});
108+
109+
it('should fail if string to array conversion is removed', async () => {
110+
baseClass = new BaseClass({
111+
log: logMock,
112+
exit: exitMock,
113+
config: {
114+
variableType: 'Skip adding environment variables',
115+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
116+
},
117+
} as any);
118+
119+
await baseClass.handleEnvImportFlow();
120+
121+
expect(exitMock).not.toHaveBeenCalled();
122+
expect(logMock).not.toHaveBeenCalledWith(
123+
"The 'Skip adding environment variables' option cannot be combined with other environment variable options. Please choose either 'Skip adding environment variables' or one or more of the other available options.",
124+
'error',
125+
);
126+
});
127+
128+
it('should handle two options: Import from stack and Manually add variables', async () => {
129+
baseClass = new BaseClass({
130+
log: logMock,
131+
exit: exitMock,
132+
config: {
133+
variableType: ['Import variables from a stack', 'Manually add custom variables to the list'],
134+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
135+
},
136+
} as any);
137+
138+
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce();
139+
const promptForEnvValuesMock = jest.spyOn(baseClass, 'promptForEnvValues').mockResolvedValueOnce();
140+
141+
await baseClass.handleEnvImportFlow();
142+
143+
expect(importEnvFromStackMock).toHaveBeenCalled();
144+
expect(promptForEnvValuesMock).toHaveBeenCalled();
145+
expect(exitMock).not.toHaveBeenCalled();
146+
});
147+
148+
it('should handle two options: Import from stack and Import from .env.local', async () => {
149+
baseClass = new BaseClass({
150+
log: logMock,
151+
exit: exitMock,
152+
config: {
153+
variableType: ['Import variables from a stack', 'Import variables from the .env.local file'],
154+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
155+
},
156+
} as any);
157+
158+
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce();
159+
const importVariableFromLocalConfigMock = jest
160+
.spyOn(baseClass, 'importVariableFromLocalConfig')
161+
.mockResolvedValueOnce();
162+
163+
await baseClass.handleEnvImportFlow();
164+
165+
expect(importEnvFromStackMock).toHaveBeenCalled();
166+
expect(importVariableFromLocalConfigMock).toHaveBeenCalled();
167+
expect(exitMock).not.toHaveBeenCalled();
168+
});
169+
170+
it('should handle two options: Manually add and Import from .env.local', async () => {
171+
baseClass = new BaseClass({
172+
log: logMock,
173+
exit: exitMock,
174+
config: {
175+
variableType: ['Manually add custom variables to the list', 'Import variables from the .env.local file'],
176+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
177+
},
178+
} as any);
179+
180+
const promptForEnvValuesMock = jest.spyOn(baseClass, 'promptForEnvValues').mockResolvedValueOnce();
181+
const importVariableFromLocalConfigMock = jest
182+
.spyOn(baseClass, 'importVariableFromLocalConfig')
183+
.mockResolvedValueOnce();
184+
185+
await baseClass.handleEnvImportFlow();
186+
187+
expect(promptForEnvValuesMock).toHaveBeenCalled();
188+
expect(importVariableFromLocalConfigMock).toHaveBeenCalled();
189+
expect(exitMock).not.toHaveBeenCalled();
190+
});
191+
192+
it('should handle all three options: Import from stack, Manually add, and Import from .env.local', async () => {
193+
baseClass = new BaseClass({
194+
log: logMock,
195+
exit: exitMock,
196+
config: {
197+
variableType: [
198+
'Import variables from a stack',
199+
'Manually add custom variables to the list',
200+
'Import variables from the .env.local file',
201+
],
202+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
203+
},
204+
} as any);
205+
206+
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce();
207+
const promptForEnvValuesMock = jest.spyOn(baseClass, 'promptForEnvValues').mockResolvedValueOnce();
208+
const importVariableFromLocalConfigMock = jest
209+
.spyOn(baseClass, 'importVariableFromLocalConfig')
210+
.mockResolvedValueOnce();
211+
212+
await baseClass.handleEnvImportFlow();
213+
214+
expect(importEnvFromStackMock).toHaveBeenCalled();
215+
expect(promptForEnvValuesMock).toHaveBeenCalled();
216+
expect(importVariableFromLocalConfigMock).toHaveBeenCalled();
217+
expect(exitMock).not.toHaveBeenCalled();
218+
});
219+
220+
it('should fail when Skip is combined with other options', async () => {
221+
baseClass = new BaseClass({
222+
log: logMock,
223+
exit: exitMock,
224+
config: {
225+
variableType: ['Skip adding environment variables', 'Import variables from a stack'],
226+
variablePreparationTypeOptions: config.variablePreparationTypeOptions,
227+
},
228+
} as any);
229+
230+
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce(undefined);
231+
232+
await baseClass.handleEnvImportFlow();
233+
234+
expect(logMock).toHaveBeenCalledWith(
235+
"The 'Skip adding environment variables' option cannot be combined with other environment variable options. Please choose either 'Skip adding environment variables' or one or more of the other available options.",
236+
'error',
237+
);
238+
expect(exitMock).toHaveBeenCalledWith(1);
239+
expect(importEnvFromStackMock).toHaveBeenCalled();
240+
});
241+
35242
it('should exit if no options are selected', async () => {
36243
(ux.inquire as jest.Mock).mockResolvedValueOnce([]);
37244

@@ -45,21 +252,30 @@ describe('BaseClass', () => {
45252
});
46253

47254
it('should exit if "Skip adding environment variables" is selected with other options', async () => {
255+
const exitMockWithThrow = jest.fn().mockImplementation(() => {
256+
throw new Error('Exit called');
257+
});
258+
baseClass = new BaseClass({
259+
log: logMock,
260+
exit: exitMockWithThrow,
261+
config: config.variablePreparationTypeOptions,
262+
} as any);
263+
48264
const importEnvFromStackMock = jest.spyOn(baseClass, 'importEnvFromStack').mockResolvedValueOnce();
49265
(ux.inquire as jest.Mock).mockResolvedValueOnce([
50266
'Skip adding environment variables',
51267
'Import variables from a stack',
52268
]);
53269

54-
await baseClass.handleEnvImportFlow();
270+
await expect(baseClass.handleEnvImportFlow()).rejects.toThrow('Exit called');
55271

56272
expect(logMock).toHaveBeenCalledWith(
57273
"The 'Skip adding environment variables' option cannot be combined with other environment variable options. Please choose either 'Skip adding environment variables' or one or more of the other available options.",
58274
'error',
59275
);
60276

61-
expect(exitMock).toHaveBeenCalledWith(1);
62-
expect(importEnvFromStackMock).toHaveBeenCalled();
277+
expect(exitMockWithThrow).toHaveBeenCalledWith(1);
278+
expect(importEnvFromStackMock).not.toHaveBeenCalled();
63279
});
64280

65281
it('should call importEnvFromStack if "Import variables from a stack" is selected', async () => {
@@ -162,7 +378,7 @@ describe('BaseClass', () => {
162378
'Import variables from the .env.local file',
163379
]);
164380

165-
await baseClass.handleEnvImportFlow();
381+
await baseClass.handleEnvImportFlow();
166382

167383
expect(importEnvFromStackMock).toHaveBeenCalled();
168384
expect(promptForEnvValuesMock).toHaveBeenCalled();
@@ -298,4 +514,36 @@ describe('BaseClass', () => {
298514
expect(exitMock).toHaveBeenCalledWith(1);
299515
});
300516
});
517+
518+
describe('handleEnvVariables', () => {
519+
beforeEach(() => {
520+
baseClass = new BaseClass({
521+
log: logMock,
522+
exit: exitMock,
523+
config: {},
524+
} as any);
525+
});
526+
527+
afterEach(() => {
528+
jest.clearAllMocks();
529+
});
530+
531+
it('should parse environment variables from config string with various value formats including URLs', async () => {
532+
baseClass = new BaseClass({
533+
log: logMock,
534+
exit: exitMock,
535+
config: {
536+
envVariables: 'APP_ENV:prod, API_URL:https://api.example.com/v1, DB_URL:postgresql://localhost:5432/dbname',
537+
},
538+
} as any);
539+
540+
await baseClass.promptForEnvValues();
541+
542+
expect(baseClass.envVariables).toEqual([
543+
{ key: 'APP_ENV', value: 'prod' },
544+
{ key: 'API_URL', value: 'https://api.example.com/v1' },
545+
{ key: 'DB_URL', value: 'postgresql://localhost:5432/dbname' },
546+
]);
547+
});
548+
});
301549
});

0 commit comments

Comments
 (0)