Fix main generator pretend spec isolation

[justin808]

Why

The default branch is currently failing Rspec test for gem on main in the generator shard:

• Run: https://github.com/shakacode/react_on_rails/actions/runs/27869267839
• Failed jobs: rspec-package-tests (4.0, latest, generators) and rspec-package-tests (3.3, minimum, generators)
• Failed example: spec/react_on_rails/generators/install_generator_spec.rb:3717

The failure was not caused by the most recent Pro streaming PR. The same generator failure appears on the prior pushed commit and traces back to the custom Shakapacker-root generator work in #4130. A focused ordered repro showed the issue:

cd react_on_rails && bundle exec rspec \
  spec/react_on_rails/generators/install_generator_spec.rb:564 \
  spec/react_on_rails/generators/install_generator_spec.rb:590 \
  spec/react_on_rails/generators/install_generator_spec.rb:3717

Before this patch, that sequence failed because earlier examples left config/shakapacker.yml in the shared generator destination. The pretend-mode example used expect(File).not_to receive(:read), which accidentally forbade the legitimate config read used to resolve Shakapacker paths. The behavior the test meant to protect is narrower: pretend mode must not read the copied Redux Tailwind client entry, because pretend mode does not create that file.

What changed

• Adds an explicit config/shakapacker.yml fixture to the pretend-mode example.
• Allows normal File.read calls while asserting the generated Redux client entry is not read.
• Keeps the production generator code unchanged.

Validation

• git diff --check -> passed.
• cd react_on_rails && bundle exec rspec spec/react_on_rails/generators/install_generator_spec.rb:564 spec/react_on_rails/generators/install_generator_spec.rb:590 spec/react_on_rails/generators/install_generator_spec.rb:3717 -> 4 examples, 0 failures.
• cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop spec/react_on_rails/generators/install_generator_spec.rb -> 1 file inspected, no offenses.
• codex review --uncommitted -> no actionable correctness issues.
• Pre-commit hook -> trailing-newlines, Ruby autofix/RuboCop passed for the changed spec.
• Pre-push hook -> branch Ruby RuboCop and markdown-links passed.

Process triage

Process Gap Disposition: checklist+replay.

• Motivating miss: Fix generated demo path hints for custom Shakapacker roots #4130 recorded focused validation for install_generator_spec.rb:3717, but the default-branch failure required replaying that example after earlier custom-root generator examples that leave config/shakapacker.yml in the shared destination.
• Replay evidence: the ordered repro above failed on main before this patch and passed after this patch.
• Non-goal: do not add a broad prose-only rule or a full generator-suite requirement for every small generator PR.

Merge queue would reduce stale-base/concurrency merges, but it would not have caught this exact issue unless the queue runs the same generator shard before landing. The actionable process improvement is to include ordered replay when adding generator examples that share dummy-for-generators, especially when new examples leave Shakapacker config or other shared files behind.

---

Note

Low Risk
Test-only change to generator spec isolation; runtime install/generator behavior is unchanged.

Overview
Fixes a flaky failure in install_generator_spec for the Redux + Tailwind --pretend example when earlier examples leave config/shakapacker.yml in the shared dummy-for-generators destination.

The example now seeds its own minimal Shakapacker config, stubs File.read with and_call_original, and asserts only that the generated HelloWorldApp.client.jsx path is not read—matching the generator’s early return in pretend mode before it would read the copied client entry. No generator production code changes.

^{Reviewed by Cursor Bugbot for commit ace8804. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Tests
  • Enhanced generator test precision for improved reliability in verifying expected file handling behavior during generator operations.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 56681396-75aa-4bdb-bfea-c93af4c05a8e

📥 Commits

Reviewing files that changed from the base of the PR and between 19a01db and ace8804.

📒 Files selected for processing (1)

• react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb

---

Walkthrough

The pretend-mode Redux+Tailwind spec is updated to create a minimal config/shakapacker.yml fixture in the generator destination, relax the File.read stub to and_call_original, and replace the blanket not_to receive(:read) assertion with a path-specific negative expectation scoped to the Redux Tailwind client entry file (HelloWorldApp.client.jsx).

Changes

Pretend-mode spec precision fix

Layer / File(s)	Summary
Scoped File.read assertion in pretend-mode test react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb	Adds a config/shakapacker.yml fixture under the generator destination root, switches the File.read stub to and_call_original so other reads proceed normally, and tightens the assertion to verify File.read is not called for the specific copied Redux Tailwind client entry path.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• shakacode/react_on_rails#3937: Modifies the same spec file to tighten pretend-mode Tailwind/Redux behavior around the HelloWorldApp.client.jsx entry file not being read.
• shakacode/react_on_rails#4130: Updates install generator path and config logic based on config/shakapacker.yml for custom Shakapacker roots, directly related to the fixture setup added in this spec fix.

Suggested labels

review-needed, P2

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/fix-main-generator-pretend-spec

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[justin808]

+ci-run-hosted

[github-actions]

Hosted CI Requested

Triggered 9 workflow(s) for ace88048d62a.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

[claude]

Code Review: Fix main generator pretend spec isolation

Summary: Clean, targeted fix for a CI regression caused by test ordering sensitivity in the shared dummy-for-generators destination. No production code changes.

What the fix does

The "does not read the copied Redux Tailwind entry in pretend mode" example at line 3717 failed when run after earlier before(:all) contexts (lines 564, 590) that call simulate_preinstalled_shakapacker, which writes config/shakapacker.yml to the shared destination. The generator's Shakapacker path resolution then tries to read that file — but the old test stub (expect(File).not_to receive(:read)) forbade all File.read calls, not just the one it actually cared about.

The fix:

1. Seeds a minimal config/shakapacker.yml so the generator can always resolve its Shakapacker paths in this test, regardless of what previous examples left behind.
2. Replaces the over-broad stub with allow(File).to receive(:read).and_call_original + a targeted expect(File).not_to receive(:read).with(File.join(destination_root, client_entry)), asserting only what matters: the not-yet-copied Redux Tailwind client entry must not be read in pretend mode.

Assessment

• Correct: The narrowed assertion exactly captures the behavior-under-test. The original not_to receive(:read) was an over-specification that accidentally coupled the test to unrelated reads.
• Minimal: Production code is untouched. The minimal YAML fixture (source_path: app/javascript) is appropriate — only the source path lookup is needed for this code path.
• Validated: The ordered repro (spec:564 spec:590 spec:3717) confirms the fix before and after.

Minor observation (not a blocker)

The describe "--pretend mode behavior" block does not have a before { prepare_destination } guard. Since it now writes config/shakapacker.yml via simulate_existing_file, any test running later in the same destination could pick up that file. Since this block uses let (not before(:all)), the file is only written when the specific example runs — so the blast radius is limited. The root isolation improvement would be to add before { prepare_destination } to this describe block, but that is outside the scope of this targeted fix.

LGTM. Ready to merge.

[claude]

The minimal YAML here (only source_path, no source_entry_path or other keys) is intentional and correct for this test — only the source path lookup is exercised. For reference, simulate_preinstalled_shakapacker in the support helper uses a full fixture with all keys. The difference is fine here since copy_base_files in pretend mode only needs to resolve the source path.

[claude]

This is the key correctness fix: and_call_original lets File.read work normally for any path, while the expect(...).not_to receive(:read).with(...) below still catches accidental reads of the specific client entry. Previously the blanket not_to receive(:read) over-specified the test and conflated "shakapacker config reads" (legitimate) with "not-yet-copied client entry reads" (the actual invariant being guarded).

[claude]

Code Review: Fix main generator pretend spec isolation

Summary: This is a small, focused test-only fix that correctly narrows an overly broad File.read stub to prevent flaky failures caused by shared state between generator spec examples.

---

What changed

The fix has three parts:

1. Adds an explicit simulate_existing_file("config/shakapacker.yml", ...) fixture so the example is self-sufficient regardless of what runs before it.
2. Adds allow(File).to receive(:read).and_call_original to permit legitimate reads (e.g., reading that Shakapacker config).
3. Narrows the blocking assertion from expect(File).not_to receive(:read) (blocks all reads) to expect(File).not_to receive(:read).with(File.join(destination_root, client_entry)) (blocks only the specific generated client entry).

---

Analysis

Correctness — correct fix

The root cause is clear: the shared dummy-for-generators destination is not wiped between examples, so earlier tests can deposit config/shakapacker.yml. The generator reads that file to resolve Shakapacker paths; the old not_to receive(:read) blocked it indiscriminately and made the test order-dependent. The new constraint — block only reads of the specific generated client entry — matches the actual intent of the test.

RSpec pattern — idiomatic

allow(File).to receive(:read).and_call_original followed by a narrowed expect(File).not_to receive(:read).with(...) is the correct RSpec approach for "allow all except this one call". No concerns here.

Minimal YAML fixture

The added fixture only includes source_path and an empty development: anchor merge. Other shakapacker.yml fixtures in this file include source_entry_path, public_output_path, etc. This minimalism is intentional — the validation confirms the test passes with just source_path — but if the generator code path ever reads additional keys the test will need updating. A brief inline comment noting why only source_path is included would help future contributors avoid unnecessary expansion.

Test isolation — correctly improved

Adding the explicit simulate_existing_file call makes this example self-sufficient regardless of what runs before it. The destination_root cleanup appears to be handled by the framework-level before/after hooks already in place. No issues found here.

No production code changed — the fix is entirely in the spec file.

---

Minor suggestions

1. The minimal YAML could benefit from a short inline comment explaining why only source_path is present (e.g., # generator only reads source_path from this config in the pretend path). This prevents future editors from "completing" the fixture with unnecessary fields.

2. The PR description documents the specific three-example ordered repro. It's worth confirming the test also passes in reverse order or in isolation, but the author's focused repro covers the known failure path and the fix is logically sound for all orderings.

---

Verdict

Approve. The fix is correct, minimal, and well-explained. The production generator is untouched. No security or performance concerns.

[greptile-apps]

Greptile Summary

Fixes a flaky CI failure in the generator shard by tightening the pretend-mode Redux + Tailwind example in install_generator_spec.rb; no production generator code is touched.

• Seeds config/shakapacker.yml with a minimal standard fixture so the test no longer depends on files left behind by earlier examples in the shared dummy-for-generators destination.
• Replaces the overly broad expect(File).not_to receive(:read) with allow(File).to receive(:read).and_call_original plus a targeted expect(File).not_to receive(:read).with(File.join(destination_root, client_entry)), correctly allowing config reads while still asserting the copied Redux Tailwind client entry is never read in pretend mode.

Confidence Score: 5/5

Test-only change that fixes a genuine CI flake without modifying any production generator code.

The change is confined to one spec file and makes two targeted, correct improvements: adding an explicit fixture to break the shared-destination dependency, and narrowing the File.read stub/expectation to match the actual behavior under test. No generator logic was altered, and the ordered repro described in the PR confirms the fix resolves the failure.

No files require special attention. The single changed file is a spec.

Important Files Changed

Filename	Overview
react_on_rails/spec/react_on_rails/generators/install_generator_spec.rb	Pretend-mode test tightened: seeds config/shakapacker.yml fixture and narrows File.read expectation to the specific Redux Tailwind client entry path instead of blocking all reads

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test starts: pretend-mode Redux+Tailwind] --> B[simulate_existing_file config/shakapacker.yml]
    B --> C[allow File.read → and_call_original]
    C --> D[expect File.read NOT called with client_entry path]
    D --> E[redux_generator.send :copy_base_files]
    E --> F{File.read called?}
    F -->|with config/shakapacker.yml| G[allowed — calls original ✓]
    F -->|with HelloWorldApp.client.jsx| H[expectation fires → test FAILS ✓]
    F -->|other paths| G
    G --> I[say_status :pretend expectation satisfied]
    I --> J[Test passes ✓]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Test starts: pretend-mode Redux+Tailwind] --> B[simulate_existing_file config/shakapacker.yml]
    B --> C[allow File.read → and_call_original]
    C --> D[expect File.read NOT called with client_entry path]
    D --> E[redux_generator.send :copy_base_files]
    E --> F{File.read called?}
    F -->|with config/shakapacker.yml| G[allowed — calls original ✓]
    F -->|with HelloWorldApp.client.jsx| H[expectation fires → test FAILS ✓]
    F -->|other paths| G
    G --> I[say_status :pretend expectation satisfied]
    I --> J[Test passes ✓]

Loading

_{Reviews (1): Last reviewed commit: "Fix pretend generator spec isolation" | Re-trigger Greptile}

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_009b06ef-c1c8-4aae-b207-87d43a6bc0b5)