Fix generator tests #1706
Fix generator tests #1706
Conversation
WalkthroughThis pull request updates documentation and scripts to enhance clarity and robustness. In the documentation file, test commands are updated, and a typo is corrected. The generator code now parses and updates the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
lib/generators/react_on_rails/dev_tests_generator.rb (1)
49-54: Improved JSON handling with proper parsing instead of string manipulation.This is a significant improvement in how the package.json file is modified. The previous implementation (not shown) likely used string replacement which can be brittle for JSON files. The new approach:
- Properly parses the JSON file
- Ensures the "scripts" object exists (initializing if needed)
- Sets the postinstall script to use Yalc for linking
- Writes back the JSON with proper formatting
This change makes the generator more robust and less prone to errors when modifying package.json.
Consider adding error handling for the JSON parsing operation, as malformed JSON in the package.json file would cause the generator to fail:
def add_yarn_relative_install_script_in_package_json package_json = File.join(destination_root, "package.json") - contents = JSON.parse(File.read(package_json)) - contents["scripts"] ||= {} - contents["scripts"]["postinstall"] = "yalc link react-on-rails" - File.open(package_json, "w+") { |f| f.puts JSON.pretty_generate(contents) } + begin + contents = JSON.parse(File.read(package_json)) + contents["scripts"] ||= {} + contents["scripts"]["postinstall"] = "yalc link react-on-rails" + File.open(package_json, "w+") { |f| f.puts JSON.pretty_generate(contents) } + rescue JSON::ParserError => e + say_status :error, "Failed to parse package.json: #{e.message}", :red + end endscript/convert (1)
4-9: Consider adding error handling to the gsub_file_content function.The
gsub_file_contentfunction is used for critical file modifications but lacks error handling for file operations and pattern matching failures.Consider adding error handling to make the script more robust:
def gsub_file_content(path, old_content, new_content) path = File.expand_path(path, __dir__) - content = File.binread(path) - content.gsub!(old_content, new_content) - File.binwrite(path, content) + begin + content = File.binread(path) + if content.match?(old_content) + content.gsub!(old_content, new_content) + File.binwrite(path, content) + puts "Updated #{path}" + else + puts "Warning: Pattern not found in #{path}" + end + rescue => e + puts "Error processing #{path}: #{e.message}" + end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CONTRIBUTING.md(1 hunks)lib/generators/react_on_rails/dev_tests_generator.rb(1 hunks)script/convert(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: examples (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build-dummy-app-webpack-test-bundles (oldest)
- GitHub Check: examples (oldest)
🔇 Additional comments (3)
CONTRIBUTING.md (1)
219-219: Updated test commands to use Shakapacker-specific targets.The command names have been appropriately updated to reference the Shakapacker-specific test targets, which aligns with the PR objective of making generator tests work with the checked-out version rather than the released version. The text also fixes a typo from "hug number" to "huge number" of files.
script/convert (2)
16-16: Improved version string matching with regex pattern.The change from a hardcoded string to a regex pattern for matching the Shakapacker gem version is a good improvement. This makes the script more flexible as it will match any version string, not just an exact version.
22-22: Consistent approach for package.json version replacement using regex.Similar to the change in line 16, this modification uses a regex pattern to match any version string for the Shakapacker npm package. This ensures consistency between how the gem and npm package versions are handled.
alexeyr-ci
force-pushed
the
alexeyr/fix-generator-tests
branch
from
e1d1c24 to
6ed25e4
Compare
Judahmeek
force-pushed
the
alexeyr/fix-generator-tests
branch
from
f2c05cd to
adf52d2
Compare
Summary
Make generator tests work with the checked-out version of React on Rails, not the last released one.
Pull Request checklist
[ ] Update documentation[ ] Update CHANGELOG fileThis change is
Summary by CodeRabbit
These improvements not only clarify developer instructions but also strengthen the update mechanisms, resulting in a more dependable and robust experience for end users.