Skip to content

Commit 8bd9359

Browse files
CopilotMossaka
andauthored
Extract environment variable parsing to testable function (#19)
* Initial plan * Refactor environment variable parsing to a testable function Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> * Address code review: eliminate regex duplication with better error handling Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Mossaka <5447827+Mossaka@users.noreply.github.com> Co-authored-by: Jiaxiao Zhou <duibao55328@gmail.com>
1 parent 59fddad commit 8bd9359

2 files changed

Lines changed: 88 additions & 49 deletions

File tree

src/cli.test.ts

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Command } from 'commander';
2+
import { parseEnvironmentVariables } from './cli';
23
import { redactSecrets } from './redact-secrets';
34
import { parseDomains } from './cli';
45

@@ -38,61 +39,69 @@ describe('cli', () => {
3839
describe('environment variable parsing', () => {
3940
it('should parse KEY=VALUE format correctly', () => {
4041
const envVars = ['GITHUB_TOKEN=abc123', 'API_KEY=xyz789'];
41-
const result: Record<string, string> = {};
42-
43-
for (const envVar of envVars) {
44-
const match = envVar.match(/^([^=]+)=(.*)$/);
45-
if (match) {
46-
const [, key, value] = match;
47-
result[key] = value;
48-
}
42+
const result = parseEnvironmentVariables(envVars);
43+
44+
expect(result.success).toBe(true);
45+
if (result.success) {
46+
expect(result.env).toEqual({
47+
GITHUB_TOKEN: 'abc123',
48+
API_KEY: 'xyz789',
49+
});
4950
}
50-
51-
expect(result).toEqual({
52-
GITHUB_TOKEN: 'abc123',
53-
API_KEY: 'xyz789',
54-
});
5551
});
5652

5753
it('should handle empty values', () => {
5854
const envVars = ['EMPTY_VAR='];
59-
const result: Record<string, string> = {};
60-
61-
for (const envVar of envVars) {
62-
const match = envVar.match(/^([^=]+)=(.*)$/);
63-
if (match) {
64-
const [, key, value] = match;
65-
result[key] = value;
66-
}
67-
}
55+
const result = parseEnvironmentVariables(envVars);
6856

69-
expect(result).toEqual({
70-
EMPTY_VAR: '',
71-
});
57+
expect(result.success).toBe(true);
58+
if (result.success) {
59+
expect(result.env).toEqual({
60+
EMPTY_VAR: '',
61+
});
62+
}
7263
});
7364

7465
it('should handle values with equals signs', () => {
7566
const envVars = ['BASE64_VAR=abc=def=ghi'];
76-
const result: Record<string, string> = {};
77-
78-
for (const envVar of envVars) {
79-
const match = envVar.match(/^([^=]+)=(.*)$/);
80-
if (match) {
81-
const [, key, value] = match;
82-
result[key] = value;
83-
}
84-
}
67+
const result = parseEnvironmentVariables(envVars);
8568

86-
expect(result).toEqual({
87-
BASE64_VAR: 'abc=def=ghi',
88-
});
69+
expect(result.success).toBe(true);
70+
if (result.success) {
71+
expect(result.env).toEqual({
72+
BASE64_VAR: 'abc=def=ghi',
73+
});
74+
}
8975
});
9076

9177
it('should reject invalid format (no equals sign)', () => {
92-
const envVar = 'INVALID_VAR';
93-
const match = envVar.match(/^([^=]+)=(.*)$/);
78+
const envVars = ['INVALID_VAR'];
79+
const result = parseEnvironmentVariables(envVars);
9480

95-
expect(match).toBeNull();
81+
expect(result.success).toBe(false);
82+
if (!result.success) {
83+
expect(result.invalidVar).toBe('INVALID_VAR');
84+
}
85+
});
86+
87+
it('should handle empty array', () => {
88+
const envVars: string[] = [];
89+
const result = parseEnvironmentVariables(envVars);
90+
91+
expect(result.success).toBe(true);
92+
if (result.success) {
93+
expect(result.env).toEqual({});
94+
}
95+
});
96+
97+
it('should return error on first invalid entry', () => {
98+
const envVars = ['VALID_VAR=value', 'INVALID_VAR', 'ANOTHER_VALID=value2'];
99+
const result = parseEnvironmentVariables(envVars);
100+
101+
expect(result.success).toBe(false);
102+
if (!result.success) {
103+
expect(result.invalidVar).toBe('INVALID_VAR');
104+
}
96105
});
97106
});
98107

src/cli.ts

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,39 @@ function redactSecrets(command: string): string {
4848
.replace(/\b(gh[pousr]_[a-zA-Z0-9]{36,255})/g, '***REDACTED***');
4949
}
5050

51+
/**
52+
* Result of parsing environment variables
53+
*/
54+
export interface ParseEnvResult {
55+
success: true;
56+
env: Record<string, string>;
57+
}
58+
59+
export interface ParseEnvError {
60+
success: false;
61+
invalidVar: string;
62+
}
63+
64+
/**
65+
* Parses environment variables from an array of KEY=VALUE strings
66+
* @param envVars Array of environment variable strings in KEY=VALUE format
67+
* @returns ParseEnvResult with parsed key-value pairs on success, or ParseEnvError with the invalid variable on failure
68+
*/
69+
export function parseEnvironmentVariables(envVars: string[]): ParseEnvResult | ParseEnvError {
70+
const result: Record<string, string> = {};
71+
72+
for (const envVar of envVars) {
73+
const match = envVar.match(/^([^=]+)=(.*)$/);
74+
if (!match) {
75+
return { success: false, invalidVar: envVar };
76+
}
77+
const [, key, value] = match;
78+
result[key] = value;
79+
}
80+
81+
return { success: true, env: result };
82+
}
83+
5184
const program = new Command();
5285

5386
program
@@ -118,17 +151,14 @@ program
118151
}
119152

120153
// Parse additional environment variables from --env flags
121-
const additionalEnv: Record<string, string> = {};
154+
let additionalEnv: Record<string, string> = {};
122155
if (options.env && Array.isArray(options.env)) {
123-
for (const envVar of options.env) {
124-
const match = envVar.match(/^([^=]+)=(.*)$/);
125-
if (!match) {
126-
logger.error(`Invalid environment variable format: ${envVar} (expected KEY=VALUE)`);
127-
process.exit(1);
128-
}
129-
const [, key, value] = match;
130-
additionalEnv[key] = value;
156+
const parsed = parseEnvironmentVariables(options.env);
157+
if (!parsed.success) {
158+
logger.error(`Invalid environment variable format: ${parsed.invalidVar} (expected KEY=VALUE)`);
159+
process.exit(1);
131160
}
161+
additionalEnv = parsed.env;
132162
}
133163

134164
const config: WrapperConfig = {

0 commit comments

Comments
 (0)