Fix integration tests for immediate_hydration Pro license changes #1783
Conversation
Mock Pro license validation per-test using around blocks. Follows existing helper spec patterns. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@ihabadham has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (1)
WalkthroughAdded RSpec around blocks in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant RSpec as RSpec Example
participant Around as Around Block
participant Config as ReactOnRails::Config
participant Utils as ReactOnRails::Utils
participant App as Test App (Turbo/Turbolinks)
RSpec->>Around: start example
activate Around
Around->>Config: set immediate_hydration = true (save previous)
Around->>Utils: stub react_on_rails_pro_licence_valid? => true
RSpec->>App: navigate / trigger Turbo/Turbolinks actions
App->>Config: hydrate React component immediately
Around->>Config: restore previous immediate_hydration
Around-->>RSpec: finish example
deactivate Around
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
Code Review for PR #1783SummaryThis PR effectively fixes two integration tests that were broken by the immediate_hydration Pro license requirements introduced in PR #1781. The solution is clean and follows established patterns in the codebase. ✅ Strengths
🔍 Observations
💡 Suggestions
shared_context 'with pro features and immediate hydration' do
before do
allow(ReactOnRails::Utils).to receive(:react_on_rails_pro_licence_valid?).and_return(true)
end
around do |example|
ReactOnRails.configure { |config| config.immediate_hydration = true }
example.run
ReactOnRails.configure { |config| config.immediate_hydration = false }
end
endThen use it as:
✅ Security & Performance
✅ Test Coverage
ConclusionThis is a solid fix that addresses the immediate issue. The code is correct and follows existing patterns. The suggested improvements are optional enhancements for better maintainability. LGTM! 👍 |
Code Review for PR #1783✅ SummaryThis PR appropriately fixes the integration tests that were broken by the immediate_hydration Pro license requirements introduced in PR #1781. The fix is well-targeted and follows existing patterns in the codebase. 👍 Positive Aspects
🔍 Observations & Minor Suggestions
✅ Security & Performance
✅ Test Coverage
🎯 Overall AssessmentApproved - This is a solid fix that addresses the immediate issue while maintaining code consistency. The minor suggestions above are optional improvements for consideration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
spec/dummy/spec/system/integration_spec.rb (3)
92-96: Restore previous config and ensure cleanup in around hookHard‑resetting to
falserisks leaking state if the default isn’tfalseand if an example fails mid‑run. Capture the prior value and restore it in an ensure block.If specs run in parallel, global config toggling can still cause flakes. Confirm parallelization is off for this suite.
-around do |example| - ReactOnRails.configure { |config| config.immediate_hydration = true } - example.run - ReactOnRails.configure { |config| config.immediate_hydration = false } -end +around do |example| + previous = ReactOnRails.configuration.immediate_hydration + ReactOnRails.configure { |config| config.immediate_hydration = true } + begin + example.run + ensure + ReactOnRails.configure { |config| config.immediate_hydration = previous } + end +end
111-115: Repeat of around hook: DRY and safe restoreSame concern as above. Use the safer pattern and consider extracting a shared_context to avoid duplication across describes.
-around do |example| - ReactOnRails.configure { |config| config.immediate_hydration = true } - example.run - ReactOnRails.configure { |config| config.immediate_hydration = false } -end +around do |example| + previous = ReactOnRails.configuration.immediate_hydration + ReactOnRails.configure { |config| config.immediate_hydration = true } + begin + example.run + ensure + ReactOnRails.configure { |config| config.immediate_hydration = previous } + end +end
118-118: Confirm predicate name and extract test helperPredicate is correct — defined as react_on_rails_pro_licence_valid? in lib/react_on_rails/utils.rb — and is stubbed in multiple specs (spec/dummy/spec/system/integration_spec.rb:99,118 and spec/dummy/spec/helpers/react_on_rails_helper_spec.rb). Extract a small helper in spec/support (e.g., stub_pro_licence!(value = true)) and include it where needed to DRY the stubs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
spec/dummy/spec/system/integration_spec.rb(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
Use RuboCop for all Ruby code
Files:
spec/dummy/spec/system/integration_spec.rb
🧠 Learnings (5)
📓 Common learnings
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-07-27T10:08:35.868Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
before do
original_value = ENV["VARIABLE_NAME"]
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
end
after do
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
ENV["VARIABLE_NAME"] = original_value
end
it "tests something" do
# Your test code here
end
end
```
This practice ensures test isolation and reliability.
Learnt from: justin808
PR: shakacode/react_on_rails#1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
before do
original_value = ENV["VARIABLE_NAME"]
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
end
after do
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
ENV["VARIABLE_NAME"] = original_value
end
it "tests something" do
# Your test code here
end
end
```
This practice ensures test isolation and reliability.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
spec/dummy/spec/system/integration_spec.rb
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
spec/dummy/spec/system/integration_spec.rb
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
spec/dummy/spec/system/integration_spec.rb
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
spec/dummy/spec/system/integration_spec.rb
🧬 Code graph analysis (1)
spec/dummy/spec/system/integration_spec.rb (2)
lib/react_on_rails/configuration.rb (1)
configure(6-9)lib/react_on_rails/react_component/render_options.rb (1)
immediate_hydration(98-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: build
🔇 Additional comments (1)
spec/dummy/spec/system/integration_spec.rb (1)
99-99: Ignore — 'licence' spelling is correct hereThe predicate is defined at lib/react_on_rails/utils.rb:216 and referenced in specs (e.g. spec/dummy/spec/system/integration_spec.rb:99, 118); no change required.
Likely an incorrect or invalid review comment.
Code Review for PR #1783: Fix integration tests for immediate_hydration Pro license changesSummaryThis PR addresses test failures caused by the introduction of Pro license requirements for the immediate_hydration feature in PR #1781. The solution implements proper mocking and configuration management for two previously skipped system tests. ✅ Strengths
🔍 Observations & Suggestions
|
Code Review for PR #1783: Fix integration tests for immediate_hydration Pro license changes🎯 Overall AssessmentThis PR effectively fixes the two integration tests that were broken by PR #1781's Pro license validation requirements. The solution is clean, well-structured, and follows existing patterns in the codebase. ✅ Strengths
🔍 Code Quality ObservationsBest Practices Followed:
Testing Approach:
|
3 participants
Summary
Fix two integration tests broken by immediate_hydration Pro license requirements in PR #1781. The issue was Pro license validation blocking immediate hydration behavior which tests relies on as being default.
Pull Request checklist
Add/update test to cover these changes(tests already exist, just fixed)Update documentationUpdate CHANGELOG fileOther Information
Uses per-test mocking approach consistent with existing helper specs. Both tests now pass reliably.
This change is
Summary by CodeRabbit