Skip to content

Commit a056ad0

Browse files
ihabadhamclaude
ihabadham
and
claude
authored
Fix brittle positional assertions in create-app tests (#2923)
## Summary - Replace `toHaveBeenNthCalledWith` ordinals with `toHaveBeenCalledWith` (semantic argument matching) - Replace `mockImplementationOnce` chains with command-matching `mockImplementation` - Replace `toHaveBeenCalledTimes` with `stepCallSummaries()` — a filtered sequence assertion on non-git commands that verifies ordering, count, and argument correctness in one check - Add `stepCallSummaries()` helper (mirrors the existing `gitCommitSubjects()` pattern for commit ordering) ## Context PR #2849 added educational git commits to the `create-react-on-rails-app` scaffold flow, interleaving `git add` + `git commit` calls between each real step. The existing tests were updated to use new ordinal positions (1→4→7→10) and counts (9, 12), but a reviewer flagged this as brittle and it was deferred as item 1 of #2888. The brittleness was verified: adding a single `execLiveArgs` call to `create-app.ts` broke 13 of 78 tests — 7 from shifted ordinals, 5 from `mockImplementationOnce` chain off-by-one, and 1 from a count mismatch. ## Approach The test file already had `gitCommitSubjects().toEqual([...])` which verifies commit ordering through `mock.calls` filtering — the [standard Jest community pattern](jestjs/jest#4402) for asserting call order on a subset of calls. This PR extends that pattern to step commands via `stepCallSummaries()`, which filters out educational git operations and asserts on the exact sequence of real steps: ```typescript expect(stepCallSummaries()).toEqual([ 'rails new', 'bundle add react_on_rails', 'bundle add react_on_rails_pro', 'bundle generate', ]); ``` This provides ordering verification (via `toEqual` on an ordered array), count verification (exact array length), and resilience to educational-commit interleaving — without the positional coupling of the original ordinal assertions. Partially addresses #2888. ## Test plan - [x] All 78 existing tests pass - [x] Adding an extra `git` command to `create-app.ts` → 0 test failures (correctly tolerant of git interleaving) - [x] Adding an extra non-git command to `create-app.ts` → `stepCallSummaries()` catches the sequence mismatch - [x] Inserting a command at the wrong position → `stepCallSummaries()` catches the ordering violation - [x] Pre-commit hooks pass (prettier, eslint, trailing-newlines) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Improved test reliability and maintainability for the app-creation flow by adding helpers to summarize and verify ordered setup steps, shifting assertions from strict call-count/order to presence and expected step sequences, and consolidating failure-path mocks for clearer, more robust error simulations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ea72c13 commit a056ad0

1 file changed

Lines changed: 77 additions & 87 deletions

File tree

‎packages/create-react-on-rails-app/tests/create-app.test.ts‎

Lines changed: 77 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,24 @@ describe('createApp', () => {
273273
});
274274
}
275275

276+
// Returns only the "step" commands (rails, bundle, pnpm, etc.) — excludes
277+
// educational git operations (add, commit) so assertions are resilient to
278+
// changes in the educational-commit interleaving.
279+
function stepCalls(): Array<[string, ...unknown[]]> {
280+
return mockedExecLiveArgs.mock.calls.filter(([cmd]) => cmd !== 'git');
281+
}
282+
283+
function stepCallSummaries(): string[] {
284+
return stepCalls().map(([cmd, args]) => {
285+
const subArgs = args as string[];
286+
if (cmd === 'rails') return `rails ${subArgs[0]}`;
287+
if (cmd === 'bundle' && subArgs[0] === 'add') return `bundle add ${subArgs[1]}`;
288+
if (cmd === 'bundle' && subArgs.includes('generate')) return 'bundle generate';
289+
if (cmd === 'pnpm') return `pnpm ${subArgs[0]}`;
290+
return `${cmd} ${subArgs[0]}`;
291+
});
292+
}
293+
276294
function expectFallbackGitIdentityOnCommits(): void {
277295
for (const [, args, , env] of gitCommitCalls()) {
278296
expect(args).toEqual(expect.arrayContaining(['-c', 'commit.gpgsign=false', 'commit']));
@@ -323,22 +341,16 @@ describe('createApp', () => {
323341

324342
createApp('my-app', options);
325343

326-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(1, 'rails', [
344+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('rails', [
327345
'new',
328346
'my-app',
329347
'--database=postgresql',
330348
'--skip-javascript',
331349
'--skip-git',
332350
]);
333-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
334-
4,
335-
'bundle',
336-
['add', 'react_on_rails', '--strict'],
337-
appPath,
338-
);
339-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(7, 'bundle', ['add', 'react_on_rails_pro'], appPath);
340-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
341-
10,
351+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails', '--strict'], appPath);
352+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails_pro'], appPath);
353+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
342354
'bundle',
343355
[
344356
'exec',
@@ -353,7 +365,12 @@ describe('createApp', () => {
353365
appPath,
354366
expect.objectContaining({ REACT_ON_RAILS_PACKAGE_MANAGER: 'npm' }),
355367
);
356-
expect(mockedExecLiveArgs).toHaveBeenCalledTimes(12);
368+
expect(stepCallSummaries()).toEqual([
369+
'rails new',
370+
'bundle add react_on_rails',
371+
'bundle add react_on_rails_pro',
372+
'bundle generate',
373+
]);
357374
expect(mockedLogStepDone).toHaveBeenCalledWith('react_on_rails gem added');
358375
expect(mockedLogStepDone).toHaveBeenCalledWith('react_on_rails_pro gem added');
359376
expect(mockedLogInfo).toHaveBeenCalledWith('Then visit http://localhost:3000');
@@ -375,22 +392,16 @@ describe('createApp', () => {
375392

376393
createApp('my-app', options);
377394

378-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(1, 'rails', [
395+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('rails', [
379396
'new',
380397
'my-app',
381398
'--database=postgresql',
382399
'--skip-javascript',
383400
'--skip-git',
384401
]);
385-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
386-
4,
387-
'bundle',
388-
['add', 'react_on_rails', '--strict'],
389-
appPath,
390-
);
391-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(7, 'bundle', ['add', 'react_on_rails_pro'], appPath);
392-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
393-
10,
402+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails', '--strict'], appPath);
403+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails_pro'], appPath);
404+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
394405
'bundle',
395406
[
396407
'exec',
@@ -405,7 +416,12 @@ describe('createApp', () => {
405416
appPath,
406417
expect.objectContaining({ REACT_ON_RAILS_PACKAGE_MANAGER: 'npm' }),
407418
);
408-
expect(mockedExecLiveArgs).toHaveBeenCalledTimes(12);
419+
expect(stepCallSummaries()).toEqual([
420+
'rails new',
421+
'bundle add react_on_rails',
422+
'bundle add react_on_rails_pro',
423+
'bundle generate',
424+
]);
409425
expect(mockedLogStepDone).toHaveBeenCalledWith('react_on_rails gem added');
410426
expect(mockedLogStepDone).toHaveBeenCalledWith('react_on_rails_pro gem added');
411427
expect(mockedLogInfo).toHaveBeenCalledWith('Then visit http://localhost:3000');
@@ -516,9 +532,8 @@ describe('createApp', () => {
516532

517533
createApp('my-app', options);
518534

519-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(7, 'bundle', ['add', 'react_on_rails_pro'], appPath);
520-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
521-
10,
535+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails_pro'], appPath);
536+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
522537
'bundle',
523538
[
524539
'exec',
@@ -557,27 +572,21 @@ describe('createApp', () => {
557572

558573
createApp('my-app', baseOptions);
559574

560-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(1, 'rails', [
575+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('rails', [
561576
'new',
562577
'my-app',
563578
'--database=postgresql',
564579
'--skip-javascript',
565580
'--skip-git',
566581
]);
567-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
568-
4,
569-
'bundle',
570-
['add', 'react_on_rails', '--strict'],
571-
appPath,
572-
);
573-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
574-
7,
582+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('bundle', ['add', 'react_on_rails', '--strict'], appPath);
583+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
575584
'bundle',
576585
['exec', 'rails', 'generate', 'react_on_rails:install', '--new-app', '--force', '--ignore-warnings'],
577586
appPath,
578587
expect.objectContaining({ REACT_ON_RAILS_PACKAGE_MANAGER: 'npm' }),
579588
);
580-
expect(mockedExecLiveArgs).toHaveBeenCalledTimes(9);
589+
expect(stepCallSummaries()).toEqual(['rails new', 'bundle add react_on_rails', 'bundle generate']);
581590
expect(mockedExecLiveArgs).not.toHaveBeenCalledWith(
582591
'bundle',
583592
['add', 'react_on_rails_pro'],
@@ -608,15 +617,14 @@ describe('createApp', () => {
608617

609618
createApp('my-app', { ...baseOptions, packageManager: 'pnpm' });
610619

611-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
612-
7,
620+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
613621
'bundle',
614622
['exec', 'rails', 'generate', 'react_on_rails:install', '--new-app', '--force', '--ignore-warnings'],
615623
appPath,
616624
expect.objectContaining({ REACT_ON_RAILS_PACKAGE_MANAGER: 'pnpm' }),
617625
);
618-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(10, 'pnpm', ['import'], appPath);
619-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(11, 'pnpm', ['install'], appPath);
626+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('pnpm', ['import'], appPath);
627+
expect(mockedExecLiveArgs).toHaveBeenCalledWith('pnpm', ['install'], appPath);
620628
expect(mockedFs.rmSync).toHaveBeenCalledWith(packageLockPath, { force: true });
621629
expect(mockedFs.writeFileSync).toHaveBeenCalledWith(
622630
packageJsonPath,
@@ -628,6 +636,13 @@ describe('createApp', () => {
628636
expect.stringContaining('system!("pnpm install")'),
629637
'utf8',
630638
);
639+
expect(stepCallSummaries()).toEqual([
640+
'rails new',
641+
'bundle add react_on_rails',
642+
'bundle generate',
643+
'pnpm import',
644+
'pnpm install',
645+
]);
631646
expect(gitCommitSubjects()).toEqual([
632647
'Create Rails app with PostgreSQL',
633648
'Add react_on_rails gem',
@@ -697,13 +712,11 @@ describe('createApp', () => {
697712

698713
it('cleans up app directory when react_on_rails add fails', () => {
699714
const appPath = path.resolve(process.cwd(), 'my-app');
700-
mockedExecLiveArgs
701-
.mockImplementationOnce(() => {})
702-
.mockImplementationOnce(() => {})
703-
.mockImplementationOnce(() => {})
704-
.mockImplementationOnce(() => {
715+
mockedExecLiveArgs.mockImplementation((command, args) => {
716+
if (command === 'bundle' && args[0] === 'add' && args[1] === 'react_on_rails') {
705717
throw new Error('ror gem install failed');
706-
});
718+
}
719+
});
707720

708721
expect(() => createApp('my-app', baseOptions)).toThrow('process.exit');
709722
expect(mockedLogError).toHaveBeenCalledWith(
@@ -763,16 +776,11 @@ describe('createApp', () => {
763776

764777
it('cleans up app directory when react_on_rails_pro add fails', () => {
765778
const appPath = path.resolve(process.cwd(), 'my-app');
766-
mockedExecLiveArgs
767-
.mockImplementationOnce(() => {})
768-
.mockImplementationOnce(() => {})
769-
.mockImplementationOnce(() => {})
770-
.mockImplementationOnce(() => {})
771-
.mockImplementationOnce(() => {})
772-
.mockImplementationOnce(() => {})
773-
.mockImplementationOnce(() => {
779+
mockedExecLiveArgs.mockImplementation((command, args) => {
780+
if (command === 'bundle' && args[0] === 'add' && args[1] === 'react_on_rails_pro') {
774781
throw new Error('pro gem install failed');
775-
});
782+
}
783+
});
776784

777785
expect(() => createApp('my-app', { ...baseOptions, rsc: true })).toThrow('process.exit');
778786
expect(mockedLogError).toHaveBeenCalledWith('Failed to add react_on_rails_pro gem required by --rsc.');
@@ -784,16 +792,11 @@ describe('createApp', () => {
784792

785793
it('cleans up app directory when react_on_rails_pro add fails for --pro', () => {
786794
const appPath = path.resolve(process.cwd(), 'my-app');
787-
mockedExecLiveArgs
788-
.mockImplementationOnce(() => {})
789-
.mockImplementationOnce(() => {})
790-
.mockImplementationOnce(() => {})
791-
.mockImplementationOnce(() => {})
792-
.mockImplementationOnce(() => {})
793-
.mockImplementationOnce(() => {})
794-
.mockImplementationOnce(() => {
795+
mockedExecLiveArgs.mockImplementation((command, args) => {
796+
if (command === 'bundle' && args[0] === 'add' && args[1] === 'react_on_rails_pro') {
795797
throw new Error('pro gem install failed');
796-
});
798+
}
799+
});
797800

798801
expect(() => createApp('my-app', { ...baseOptions, pro: true })).toThrow('process.exit');
799802
expect(mockedLogError).toHaveBeenCalledWith('Failed to add react_on_rails_pro gem required by --pro.');
@@ -804,16 +807,11 @@ describe('createApp', () => {
804807
});
805808

806809
it('falls back to manual cleanup guidance if automatic cleanup fails', () => {
807-
mockedExecLiveArgs
808-
.mockImplementationOnce(() => {})
809-
.mockImplementationOnce(() => {})
810-
.mockImplementationOnce(() => {})
811-
.mockImplementationOnce(() => {})
812-
.mockImplementationOnce(() => {})
813-
.mockImplementationOnce(() => {})
814-
.mockImplementationOnce(() => {
810+
mockedExecLiveArgs.mockImplementation((command, args) => {
811+
if (command === 'bundle' && args[0] === 'add' && args[1] === 'react_on_rails_pro') {
815812
throw new Error('pro gem install failed');
816-
});
813+
}
814+
});
817815
mockedFs.rmSync.mockImplementationOnce(() => {
818816
throw new Error('cleanup failed');
819817
});
@@ -826,16 +824,11 @@ describe('createApp', () => {
826824

827825
it('cleans up app directory when generator fails', () => {
828826
const appPath = path.resolve(process.cwd(), 'my-app');
829-
mockedExecLiveArgs
830-
.mockImplementationOnce(() => {})
831-
.mockImplementationOnce(() => {})
832-
.mockImplementationOnce(() => {})
833-
.mockImplementationOnce(() => {})
834-
.mockImplementationOnce(() => {})
835-
.mockImplementationOnce(() => {})
836-
.mockImplementationOnce(() => {
827+
mockedExecLiveArgs.mockImplementation((command, args) => {
828+
if (command === 'bundle' && args.includes('generate')) {
837829
throw new Error('generator failed');
838-
});
830+
}
831+
});
839832

840833
expect(() => createApp('my-app', baseOptions)).toThrow('process.exit');
841834
expect(mockedLogError).toHaveBeenCalledWith(
@@ -852,8 +845,7 @@ describe('createApp', () => {
852845

853846
createApp('my-app', baseOptions);
854847

855-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
856-
4,
848+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
857849
'bundle',
858850
['add', 'react_on_rails', '--path', localGemPath],
859851
appPath,
@@ -867,8 +859,7 @@ describe('createApp', () => {
867859

868860
createApp('my-app', { ...baseOptions, rsc: true });
869861

870-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
871-
7,
862+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
872863
'bundle',
873864
['add', 'react_on_rails_pro', '--path', localProGemPath],
874865
appPath,
@@ -882,8 +873,7 @@ describe('createApp', () => {
882873

883874
createApp('my-app', { ...baseOptions, pro: true });
884875

885-
expect(mockedExecLiveArgs).toHaveBeenNthCalledWith(
886-
7,
876+
expect(mockedExecLiveArgs).toHaveBeenCalledWith(
887877
'bundle',
888878
['add', 'react_on_rails_pro', '--path', localProGemPath],
889879
appPath,

0 commit comments

Comments
 (0)