Add cypress-rails compatible rake tasks and server lifecycle hooks #179
Conversation
This enhancement brings cypress-rails functionality to cypress-playwright-on-rails: - Added rake tasks for cypress:open, cypress:run, playwright:open, playwright:run - Implemented automatic Rails server management with dynamic port selection - Added server lifecycle hooks (before_server_start, after_server_start, etc.) - Added transactional test mode for automatic database rollback - Added state reset middleware for /cypress_rails_reset_state endpoint - Support for CYPRESS_RAILS_HOST and CYPRESS_RAILS_PORT environment variables These changes make cypress-playwright-on-rails a more complete replacement for cypress-rails, providing the same developer-friendly test execution experience while maintaining all the existing cypress-on-rails functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 25 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 (2)
WalkthroughAdds rake tasks and a new Server class to manage Cypress/Playwright test runs with automatic Rails server lifecycle, transactional mode, and env setup. Introduces state reset middleware and configurable lifecycle hooks. Updates configuration API, railtie wiring, initializer template comments, and documentation (README and CHANGELOG). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Rake as Rake Task
participant Srv as CypressOnRails::Server
participant Rails as Rails App Server
participant Test as Cypress/Playwright
Dev->>Rake: rake cypress:open / cypress:run / playwright:*
Rake->>Srv: new(...).open()/run()
note over Srv: before_server_start hook
Srv->>Rails: spawn (host/port/env)
Srv-->>Srv: wait until ready (timeout)
note over Srv: after_server_start hook
alt transactional_server
Srv-->>Rails: begin transaction (AR)
note over Srv: after_transaction_start hook
end
Srv->>Test: exec command (open/run)
Test-->>Srv: exit status
note over Srv: before_server_stop hook
opt transactional_server
Srv-->>Rails: rollback transaction
end
Srv-->>Rails: terminate server
Srv-->>Dev: exit with status
sequenceDiagram
autonumber
participant Client as Test Runner / Helper
participant MW as StateResetMiddleware
participant App as Rails App
participant DB as DB/AR
participant Cache as Rails.cache
Client->>MW: HTTP POST /__cypress__/reset (example)
alt matches reset path
note over MW: after_state_reset hook (safe invoke)
MW-->>DB: clean DB (DatabaseCleaner or truncate/clear)
MW-->>Cache: clear cache (if available)
MW-->>App: clear dependencies
MW-->>Client: 200 OK (reset done)
else non-reset path
MW->>App: call(env)
App-->>Client: response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Added detailed migration instructions for: - Users currently using manual server management (old way) - Users migrating from cypress-rails gem The migration guide clearly shows the before/after comparison and provides step-by-step instructions for both scenarios. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)README.md(2 hunks)lib/cypress_on_rails/configuration.rb(2 hunks)lib/cypress_on_rails/railtie.rb(1 hunks)lib/cypress_on_rails/server.rb(1 hunks)lib/cypress_on_rails/state_reset_middleware.rb(1 hunks)lib/generators/cypress_on_rails/templates/config/initializers/cypress_on_rails.rb.erb(1 hunks)lib/tasks/cypress.rake(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
lib/cypress_on_rails/state_reset_middleware.rb (2)
lib/cypress_on_rails/configuration.rb (2)
initialize(36-38)configuration(76-78)lib/cypress_on_rails/server.rb (2)
initialize(9-18)run_hook(190-194)
lib/cypress_on_rails/railtie.rb (1)
lib/cypress_on_rails/configuration.rb (1)
configuration(76-78)
lib/cypress_on_rails/server.rb (2)
lib/cypress_on_rails/configuration.rb (2)
initialize(36-38)configuration(76-78)lib/cypress_on_rails/state_reset_middleware.rb (4)
initialize(2-44)initialize(3-5)run_hook(41-43)call(7-14)
🪛 Brakeman (7.0.2)
lib/cypress_on_rails/state_reset_middleware.rb
[medium] 30-30: Possible SQL injection
Type: SQL Injection
Confidence: Medium
More info: https://brakemanscanner.org/docs/warning_types/sql_injection/
(SQL Injection)
lib/cypress_on_rails/server.rb
[medium] 187-187: Possible command injection
Type: Command Injection
Confidence: Medium
More info: https://brakemanscanner.org/docs/warning_types/command_injection/
(Command Injection)
[medium] 107-107: Possible command injection
Type: Command Injection
Confidence: Medium
More info: https://brakemanscanner.org/docs/warning_types/command_injection/
(Command Injection)
| # Check common locations for cypress/playwright installation | ||
| possible_folders = ['e2e', 'spec/e2e', 'spec/cypress', 'spec/playwright', 'cypress', 'playwright'] | ||
| folder = possible_folders.find { |f| File.exist?(f) } | ||
| folder || 'e2e' |
There was a problem hiding this comment.
Should we follow a similar pattern from config using File.expand_path?
| end | ||
| end | ||
| end | ||
| end No newline at end of file |
| end | ||
|
|
||
| # Clear Rails cache | ||
| Rails.cache.clear if defined?(Rails) && Rails.cache |
There was a problem hiding this comment.
Why does run_hook(config.after_state_reset) go above the reset?
Seems like this clear cache line should be here, so we don't need to have it in the hook by default
c.after_state_reset = -> { Rails.cache.clear }
We can have some puts by default
# c.after_state_reset = -> { puts "State reset is done" }
|
@claude please review this PR |
|
Claude finished @justin808's task —— View job Code Review Complete ✅Todo List:
Overall AssessmentThis PR adds valuable cypress-rails compatible functionality, making the gem a more complete replacement for cypress-rails. The rake tasks and server lifecycle hooks will significantly improve the developer experience. However, there are several critical security issues and bugs that must be addressed before merging. Critical Issues
|
- Add require 'fileutils' to server.rb - Refactor shell command execution to prevent injection attacks - Use argument arrays instead of string interpolation - Pass commands as arrays to spawn() and system() - Improve database state reset safety - Move after_state_reset hook to run after cleanup - Add support for disable_referential_integrity when available - Use proper table name quoting with quote_table_name - Use File.expand_path for folder detection These changes address security concerns about command injection and improve compatibility across different Rails environments. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Code Review for PR #179Thank you for this comprehensive PR! Here's my detailed review: Strengths
Critical Issues1. SQL Injection Risk (lib/cypress_on_rails/state_reset_middleware.rb:30-39) 2. Process Management Issues
3. Missing Test Coverage
4. Configuration Validation 5. Performance Consideration Security Notes
The main blockers are the SQL injection vulnerability and lack of test coverage. Once addressed, this will be a great addition! |
There was a problem hiding this comment.
Pull Request Overview
This PR adds cypress-rails compatible functionality to cypress-playwright-on-rails, providing rake tasks for automated test execution and server lifecycle hooks for customizing test server behavior.
- Added rake tasks (
cypress:open,cypress:run,playwright:open,playwright:run) that automatically manage Rails server lifecycle - Introduced server lifecycle hooks for customizing test environment setup and teardown
- Added state reset endpoints and transactional test mode for database management between tests
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/tasks/cypress.rake | Defines rake tasks for opening and running Cypress/Playwright tests with automatic server management |
| lib/generators/cypress_on_rails/templates/config/initializers/cypress_on_rails.rb.erb | Adds configuration template for server settings and lifecycle hooks |
| lib/cypress_on_rails/state_reset_middleware.rb | Implements middleware for resetting application state between tests with database cleanup |
| lib/cypress_on_rails/server.rb | Core server management class handling test server lifecycle, port detection, and command execution |
| lib/cypress_on_rails/railtie.rb | Integrates rake tasks and state reset middleware into Rails application |
| lib/cypress_on_rails/configuration.rb | Extends configuration with server settings and lifecycle hook attributes |
| README.md | Documents new rake tasks, server hooks configuration, and migration guidance |
| CHANGELOG.md | Records new features and provides migration guide from cypress-rails |
Comments suppressed due to low confidence (1)
lib/cypress_on_rails/state_reset_middleware.rb:1
- This line is checking
transaction_open?on the connection but appears in the server.rb file context. However, the methodtransaction_open?may not be available in all ActiveRecord versions and could raise a NoMethodError.
module CypressOnRails
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ActiveRecord::Base.connection.begin_transaction(joinable: false) | ||
| run_hook(config.after_transaction_start) | ||
| end | ||
|
|
||
| yield | ||
|
|
||
| ensure | ||
| run_hook(config.before_server_stop) | ||
|
|
||
| if @transactional && defined?(ActiveRecord::Base) | ||
| ActiveRecord::Base.connection.rollback_transaction if ActiveRecord::Base.connection.transaction_open? | ||
| end | ||
|
|
||
| stop_server(server_pid) | ||
| ENV.delete('CYPRESS') | ||
| end |
There was a problem hiding this comment.
The transaction logic could lead to issues in multi-threaded environments. The begin_transaction(joinable: false) creates a non-joinable transaction, but there's no guarantee that the same connection will be used throughout the test execution, potentially causing transaction state inconsistencies.
| ActiveRecord::Base.connection.begin_transaction(joinable: false) | |
| run_hook(config.after_transaction_start) | |
| end | |
| yield | |
| ensure | |
| run_hook(config.before_server_stop) | |
| if @transactional && defined?(ActiveRecord::Base) | |
| ActiveRecord::Base.connection.rollback_transaction if ActiveRecord::Base.connection.transaction_open? | |
| end | |
| stop_server(server_pid) | |
| ENV.delete('CYPRESS') | |
| end | |
| ActiveRecord::Base.connection_pool.with_connection do |conn| | |
| conn.begin_transaction(joinable: false) | |
| run_hook(config.after_transaction_start) | |
| begin | |
| yield | |
| ensure | |
| run_hook(config.before_server_stop) | |
| conn.rollback_transaction if conn.transaction_open? | |
| end | |
| end | |
| else | |
| yield | |
| run_hook(config.before_server_stop) | |
| end | |
| stop_server(server_pid) | |
| ENV.delete('CYPRESS') |
| end | ||
|
|
||
| def command_exists?(command) | ||
| system("which #{command} > /dev/null 2>&1") |
There was a problem hiding this comment.
The command parameter is passed directly to shell without sanitization, creating a potential command injection vulnerability. Use Open3.capture3 or validate the command parameter to ensure it contains only expected command names.
| system("which #{command} > /dev/null 2>&1") | |
| system('which', command, out: File::NULL, err: File::NULL) |
| connection.disable_referential_integrity do | ||
| connection.tables.each do |table| | ||
| next if table == 'schema_migrations' || table == 'ar_internal_metadata' | ||
| connection.execute("DELETE FROM #{connection.quote_table_name(table)}") |
There was a problem hiding this comment.
The DELETE query is duplicated in both the disable_referential_integrity block and the fallback section. Extract this logic into a private method to avoid code duplication.
| # Fallback to regular deletion with proper table name quoting | ||
| connection.tables.each do |table| | ||
| next if table == 'schema_migrations' || table == 'ar_internal_metadata' | ||
| connection.execute("DELETE FROM #{connection.quote_table_name(table)}") |
There was a problem hiding this comment.
The DELETE query is duplicated in both the disable_referential_integrity block and the fallback section. Extract this logic into a private method to avoid code duplication.
Summary
This PR adds cypress-rails compatible functionality to cypress-playwright-on-rails, making it a more complete replacement for cypress-rails.
Key Features Added
1. Rake Tasks for Test Execution
cypress:open- Opens Cypress test runner UIcypress:run- Runs Cypress tests in headless modeplaywright:open- Opens Playwright test runner UIplaywright:run- Runs Playwright tests in headless modeThese tasks automatically:
2. Server Lifecycle Hooks
New configuration hooks for customizing test server behavior:
before_server_start- Run code before Rails server startsafter_server_start- Run code after Rails server is readyafter_transaction_start- Run code after database transaction begins (transactional mode)after_state_reset- Run code after application state is resetbefore_server_stop- Run code before Rails server stops3. Additional Enhancements
/cypress_rails_reset_stateendpoint for compatibilityCYPRESS_RAILS_HOSTandCYPRESS_RAILS_PORTMigration from cypress-rails
This enhancement makes migrating from cypress-rails straightforward:
gem 'cypress-rails'withgem 'cypress-on-rails'rails g cypress_on_rails:installconfig/initializers/cypress_on_rails.rbcypress:open,cypress:run)Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation