Skip to content

Commit cd07a16

Browse files
panz3rCopilot
andauthored
test: setup E2E tests (#286)
* test: implement end-to-end tests for CLI commands and improve error handling * test: re-organize integration test files * test: enhance output handling in `runCLI` helper by stripping VT control characters * test: enhance end-to-end tests for CLI commands with additional assertions and error handling * chore(actions): separate unit, integration, and end-to-end tests in CI workflow * docs: update E2E test implementation plan to completed with comprehensive coverage * test: refactor `test/helpers/run-cli.ts` file Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 6d0affc commit cd07a16

File tree

21 files changed

+832
-53
lines changed

21 files changed

+832
-53
lines changed

.github/workflows/build-test.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,8 @@ jobs:
4242
- name: Build CLI
4343
run: pnpm run build
4444

45-
- name: Test CLI commands
46-
run: pnpm run test
45+
- name: Test CLI commands (unit & integration)
46+
run: pnpm run test:integration
47+
48+
- name: Test CLI commands (e2e)
49+
run: pnpm run test:e2e

bin/dev.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,11 @@
22

33
import { runCLI } from '../src/cli/runner.js'
44

5-
await runCLI(process.argv.slice(2))
5+
try {
6+
await runCLI(process.argv.slice(2))
7+
} catch (err) {
8+
// CommandError will call process.exit() via error() function
9+
// Other errors should exit with code 1
10+
console.error(err)
11+
process.exit(1)
12+
}

bin/run.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,11 @@
22

33
import { runCLI } from '../dist/cli/runner.js'
44

5-
await runCLI(process.argv.slice(2))
5+
try {
6+
await runCLI(process.argv.slice(2))
7+
} catch (err) {
8+
// CommandError will call process.exit() via error() function
9+
// Other errors should exit with code 1
10+
console.error(err)
11+
process.exit(1)
12+
}

docs/004-E2E-TEST-IMPLEMENTATION.md

Lines changed: 119 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Proposal: E2E Test Implementation
22

3-
**Status**: Proposed
3+
**Status**: ✅ Implemented
44
**Date**: January 11, 2026
55
**Author**: Architecture Review
66
**Priority**: Nice to Have
@@ -281,29 +281,33 @@ describe('CLI E2E', () => {
281281

282282
## Implementation Plan
283283

284-
### Phase 1: Helper & Basic Tests
284+
### Phase 1: Helper & Basic Tests ✅ COMPLETED
285285

286-
| Task | Description | Effort |
287-
|------|-------------|--------|
288-
| 1.1 | Create `test/helpers/run-cli.ts` | 30 min |
289-
| 1.2 | Create `test/e2e/cli.test.ts` with global flag tests | 30 min |
290-
| 1.3 | Add unknown command tests | 15 min |
291-
| 1.4 | Add command help tests | 15 min |
286+
| Task | Description | Effort | Status |
287+
|------|-------------|--------|--------|
288+
| 1.1 | Create `test/helpers/run-cli.ts` | 30 min | ✅ Done |
289+
| 1.2 | Create `test/e2e/cli.test.ts` with global flag tests | 30 min | ✅ Done |
290+
| 1.3 | Add unknown command tests | 15 min | ✅ Done |
291+
| 1.4 | Add command help tests | 15 min | ✅ Done |
292292

293-
### Phase 2: Exit Code Tests
293+
### Phase 2: Exit Code Tests ✅ COMPLETED
294294

295-
| Task | Description | Effort |
296-
|------|-------------|--------|
297-
| 2.1 | Add exit code tests for each error scenario | 30 min |
298-
| 2.2 | Test production build (`bin/run.js`) | 15 min |
299-
| 2.3 | Test dev build (`bin/dev.js`) | 15 min |
295+
| Task | Description | Effort | Status |
296+
|------|-------------|--------|--------|
297+
| 2.1 | Add exit code tests for each error scenario | 30 min | ✅ Done |
298+
| 2.2 | Test production build (`bin/run.js`) | 15 min | ⚠️ Deferred |
299+
| 2.3 | Test dev build (`bin/dev.js`) | 15 min | ✅ Done |
300300

301-
### Phase 3: CI Integration (Optional)
301+
> **Note**: Task 2.2 deferred - all E2E tests currently use dev mode (`dev: true`) to avoid build requirement. Production build testing can be added later if needed.
302302
303-
| Task | Description | Effort |
304-
|------|-------------|--------|
305-
| 3.1 | Add E2E test script to `package.json` | 10 min |
306-
| 3.2 | Run E2E tests in CI after build step | 15 min |
303+
### Phase 3: CI Integration ✅ COMPLETED
304+
305+
| Task | Description | Effort | Status |
306+
|------|-------------|--------|--------|
307+
| 3.1 | Add E2E test script to `package.json` | 10 min | ✅ Done |
308+
| 3.2 | Run E2E tests in CI after build step | 15 min | ⚠️ Partial |
309+
310+
> **Note**: Task 3.2 partial - E2E tests added to package.json scripts, but not yet verified in CI pipeline.
307311
308312
---
309313

@@ -386,12 +390,51 @@ const { stdout } = await runCLI(['icons'], { timeout: 60000 })
386390

387391
## Success Criteria
388392

389-
- [ ] `test/helpers/run-cli.ts` created and working
390-
- [ ] `test/e2e/cli.test.ts` with core test cases
391-
- [ ] All global flag tests passing
392-
- [ ] Exit code tests for all error scenarios
393-
- [ ] Tests work with both `bin/run.js` and `bin/dev.js`
394-
- [ ] Tests pass in CI environment
393+
- [x] `test/helpers/run-cli.ts` created and working
394+
- [x] `test/e2e/cli.test.ts` with core test cases
395+
- [x] All global flag tests passing
396+
- [x] Exit code tests for all error scenarios
397+
- [x] Tests work with `bin/dev.js`
398+
- [ ] Tests work with both `bin/run.js` and `bin/dev.js` *(deferred)*
399+
- [x] Tests pass in local environment
400+
- [ ] Tests verified in CI environment *(pending CI verification)*
401+
402+
## Additional Test Coverage Implemented
403+
404+
Beyond the original proposal, the following test suites were added:
405+
406+
### Icons Command Tests
407+
- ✅ Generate icons with default file path (`assets/icon.png`)
408+
- ✅ Generate icons with custom file path
409+
- ✅ Verbose output flag (`-v`)
410+
- ✅ Corrupt image file handling
411+
- ✅ Verify iOS `Contents.json` structure
412+
- ✅ Verify all Android density variants
413+
- ✅ Verify all iOS icon sizes
414+
415+
### Splash Command Tests
416+
- ✅ Generate splashscreens successfully
417+
- ✅ Use default file path when not specified
418+
- ✅ Verbose output
419+
- ✅ FILE_NOT_FOUND error for missing file
420+
- ✅ Verify iOS `Contents.json` structure
421+
- ✅ Verify all Android drawable densities
422+
- ✅ Verify iOS 1x/2x/3x variants
423+
424+
### Dotenv Command Tests
425+
- ✅ Copy environment file successfully
426+
- ✅ Replace existing `.env` file
427+
- ✅ Verbose output
428+
- ✅ Fail when source file doesn't exist
429+
430+
### App Name Resolution Tests
431+
- ✅ Read app name from `app.json`
432+
- ✅ Fail when `app.json` missing and no `--appName`
433+
- ✅ Prioritize `--appName` flag over `app.json`
434+
435+
### Flag Variations Tests
436+
- ✅ Short flag `-a` for `appName`
437+
- ✅ Both `--verbose` and `-v` flags work
395438

396439
---
397440

@@ -421,3 +464,54 @@ Recommended triggers:
421464
| Date | Change | Author |
422465
|------|--------|--------|
423466
| 2026-01-11 | Initial proposal (extracted from 003-OCLIF-MIGRATION-PROPOSAL.md Appendix B) | Architecture Review |
467+
| 2026-01-11 | ✅ Implementation completed - all phases done with comprehensive test coverage | Development Team |
468+
469+
---
470+
471+
## Implementation Review
472+
473+
### What Was Implemented
474+
475+
The E2E test implementation **exceeded expectations** with comprehensive coverage:
476+
477+
1. **Core Infrastructure**
478+
- `test/helpers/run-cli.ts` with support for dev/production modes, custom environment variables, timeout control, and VT control character stripping
479+
- Uses `tsx` loader for dev mode instead of relying on shebang
480+
- Proper subprocess spawning with output capture
481+
482+
2. **Test Coverage**
483+
- All proposed tests implemented
484+
- **Additional 30+ test cases** covering real-world scenarios
485+
- File verification with actual iOS/Android output structure
486+
- JSON structure validation for `Contents.json` files
487+
- Flag variation testing (short/long forms)
488+
489+
3. **Package.json Scripts**
490+
- `test:e2e` script added for isolated E2E testing
491+
- `test:integration` script for integration tests
492+
- Main `test` command runs both with coverage
493+
494+
### Key Differences from Proposal
495+
496+
1. **Enhanced `run-cli.ts` helper**:
497+
- Added `stripVTControlCharacters` from `node:util` for cleaner output assertions
498+
- Dev mode uses explicit `tsx` loader instead of shebang
499+
- Both `NO_COLOR` and `NODE_DISABLE_COLORS` environment variables set
500+
501+
2. **More comprehensive test assertions**:
502+
- Actual file existence checks for generated assets
503+
- JSON structure validation
504+
- Content verification (not just exit codes)
505+
- Cross-platform output verification (iOS + Android)
506+
507+
3. **Test organization**:
508+
- Tests grouped by command with proper setup/teardown
509+
- Temporary directory management in `tmp/e2e-tests`
510+
- Proper cleanup in `after`/`afterEach` hooks
511+
512+
### Deferred Items
513+
514+
- **Production build testing**: All tests use `dev: true` to avoid build dependency
515+
- **CI verification**: Scripts added but not yet verified in actual CI environment
516+
517+
These can be addressed in future iterations if needed.

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
"build": "tsc -b",
2323
"lint": "eslint",
2424
"test": "node --import tsx --test --test-concurrency=1 --experimental-test-coverage 'test/**/*.test.ts'",
25+
"test:integration": "node --import tsx --test --test-concurrency=1 'test/integration/**/*.test.ts'",
26+
"test:e2e": "node --import tsx --test --test-concurrency=1 'test/e2e/**/*.test.ts'",
2527
"check": "pnpm run lint && pnpm run test",
2628
"prepack": "pnpm build"
2729
},

src/cli/parser.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,17 @@ export async function parseArgs(argv: string[], config: CommandConfig): Promise<
7272
// Build args object from positionals
7373
const args: Record<string, string | undefined> = {}
7474

75+
// Skip required argument validation if help flag is present
76+
const isHelpRequested = Boolean(flags.help)
77+
7578
for (const [index, argConfig] of config.args.entries()) {
7679
const value = positionals[index]
7780

7881
if (value !== undefined) {
7982
args[argConfig.name] = value
8083
} else if (argConfig.default !== undefined) {
8184
args[argConfig.name] = argConfig.default
82-
} else if (argConfig.required) {
85+
} else if (argConfig.required && !isHelpRequested) {
8386
throw new CommandError(
8487
`Missing required argument: ${argConfig.name}`,
8588
ExitCode.INVALID_ARGUMENT,

src/cli/runner.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ export async function runCLI(argv: string[]): Promise<void> {
9595
} catch (err) {
9696
if (err instanceof CommandError) {
9797
error(err.message, err.exitCode)
98+
// error() calls process.exit(), so this line is never reached
99+
// but TypeScript doesn't know that, so we need to return or re-throw
100+
return
98101
}
99102

100103
throw err

0 commit comments

Comments
 (0)