Skip to content

Supersede #904: compiler_spec call-count refactor on clean branch#911

Closed
justin808 wants to merge 1 commit intomainfrom
codex/supersede-904-call-count
Closed

Supersede #904: compiler_spec call-count refactor on clean branch#911
justin808 wants to merge 1 commit intomainfrom
codex/supersede-904-call-count

Conversation

@justin808
Copy link
Copy Markdown
Member

@justin808 justin808 commented Feb 15, 2026

Supersedes #904. Closes #855.

Summary

Notes

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 15, 2026

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 21 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/supersede-904-call-count

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

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

@justin808 justin808 mentioned this pull request Feb 15, 2026
Closed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 15, 2026

Greptile Summary

This PR refactors test assertions in compiler_spec.rb to follow the project's style guide by replacing indirect call_count patterns with explicit RSpec spy assertions.

Key changes:

  • Replaced call_count += 1 and if call_count == 1 logic with direct argument matching (args[1] == hook_command or args[0] == hook_executable)
  • Extracted hook command strings into named variables for clarity (hook_command, hook_executable)
  • Maintained correct argument indexing based on stub block signature (|*args| vs |env, *args|)
  • No functional changes to test behavior - purely assertion style cleanup

The refactoring correctly handles the two different stub patterns:

  • When using |*args|, checks args[1] (since args[0] is the env hash)
  • When using |env, *args|, checks args[0] (since env is extracted separately)

All changes align with CLAUDE.md guidance to prefer explicit have_received assertions over indirect counter patterns.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are purely stylistic refactoring of test code with no functional impact. The refactoring correctly maintains test behavior while improving code clarity by following the project's style guidelines. All argument indexing is consistent with stub block signatures, and the logic properly distinguishes between hook and webpack calls.
  • No files require special attention

Important Files Changed

Filename Overview
spec/shakapacker/compiler_spec.rb Refactored test assertions from call-count pattern to explicit argument checking, following CLAUDE.md style guide recommendations

Last reviewed commit: 47d4302

@justin808
Copy link
Copy Markdown
Member Author

justin808 commented Feb 15, 2026

Superseded by #920, which preserves the original compiler_spec refactor and adds the lint-unblock commit.

@justin808 justin808 mentioned this pull request Feb 15, 2026
Merged
@justin808 justin808 closed this Feb 15, 2026
justin808 added a commit that referenced this pull request Mar 11, 2026
@justin808 @claude
Supersede #911: compiler_spec call-count refactor with lint unblock (#…
1f3792d 
…920)

Supersedes #911.

Includes:
- original compiler_spec call-count refactor
- .prettierignore workflow-file ignore update to avoid unrelated lint
failures

Closes #855

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Test-only refactor that changes how mocks detect the hook call;
production behavior is unchanged, with minimal risk beyond potential
spec brittleness.
> 
> **Overview**
> Updates `spec/shakapacker/compiler_spec.rb` to remove manual
`call_count` logic in several `precompile_hook` examples and instead
branch the `Open3.capture3` stub based on the invoked
command/executable, using shared `hook_command`/`hook_executable`
variables.
> 
> This makes the tests align with the real `Open3.capture3(env,
executable, *args, chdir: ...)` signature and improves robustness for
cases like quoted paths, missing executables, command chaining, and
env-var-prefixed hook commands.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
3df53f2. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Expanded test coverage for precompile hook handling:
special-character/quote cases, executable-vs-argument behavior to
prevent shell interpretation, path traversal and security checks,
environment variable extraction/merging, malformed command handling, and
skip-logic scenarios.
* **Documentation**
* Clarified triage flow to always offer optional rationale replies for
skipped/declined items, require explicit user selection before posting,
and added prompt guidance for rationale replies.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Complete refactoring of call_count patterns in compiler_spec.rb

1 participant

@justin808