Skip to content

Commit 4b2bef4

Browse files
Fix CI failures: RuboCop violations and test failures
- Fix RuboCop violations: - Use string interpolation instead of concatenation in base_generator.rb - Apply guard clause pattern in configuration.rb - Remove extra blank line in utils.rb - Fix configuration validation tests: - Add Rails.root availability check to prevent nil errors in tests - Mock Rails.root in test specs for path validation - Fix utils tests: - Use default 'ssr-generated' path instead of nil in mock_bundle_configs - Update test expectations to match new server bundle path behavior - Remove outdated test expecting server bundles in public/packs Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com>
1 parent 2a2b663 commit 4b2bef4

File tree

5 files changed

+21
-16
lines changed

5 files changed

+21
-16
lines changed

lib/generators/react_on_rails/base_generator.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ def update_gitignore_for_auto_registration
146146
append_to_file ".gitignore" do
147147
lines = ["\n# Generated React on Rails packs"]
148148
lines.concat(additions)
149-
lines.join("\n") + "\n"
149+
"#{lines.join("\n")}\n"
150150
end
151151
end
152152

lib/react_on_rails/configuration.rb

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,17 @@ def validate_enforce_secure_server_bundles
211211
end
212212

213213
# Check if server_bundle_output_path is inside public directory
214+
# Skip validation if Rails.root is not available (e.g., in tests)
215+
return unless defined?(Rails) && Rails.root
216+
214217
public_path = Rails.root.join("public").to_s
215218
server_output_path = File.expand_path(server_bundle_output_path, Rails.root.to_s)
216219

217-
if server_output_path.start_with?(public_path)
218-
raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
219-
"server_bundle_output_path (#{server_bundle_output_path}) is inside " \
220-
"the public directory. Please set it to a directory outside of public."
221-
end
220+
return unless server_output_path.start_with?(public_path)
221+
222+
raise ReactOnRails::Error, "enforce_secure_server_bundles is set to true, but " \
223+
"server_bundle_output_path (#{server_bundle_output_path}) is inside " \
224+
"the public directory. Please set it to a directory outside of public."
222225
end
223226

224227
def check_autobundling_requirements

lib/react_on_rails/utils.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@ def self.bundle_js_file_path(bundle_name)
133133
File.expand_path(File.join(ReactOnRails::PackerUtils.packer_public_output_path, bundle_name))
134134
end
135135

136-
137136
def self.server_bundle_js_file_path
138137
return @server_bundle_path if @server_bundle_path && !Rails.env.development?
139138

spec/react_on_rails/configuration_spec.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,11 @@ module ReactOnRails
428428

429429
describe "enforce_secure_server_bundles validation" do
430430
context "when enforce_secure_server_bundles is true" do
431+
before do
432+
# Mock Rails.root for tests that need path validation
433+
allow(Rails).to receive(:root).and_return(Pathname.new("/test/app"))
434+
end
435+
431436
it "raises error when server_bundle_output_path is nil" do
432437
expect do
433438
ReactOnRails.configure do |config|

spec/react_on_rails/utils_spec.rb

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def mock_bundle_configs(server_bundle_name: random_bundle_name, rsc_bundle_name:
7777
allow(ReactOnRails).to receive_message_chain("configuration.rsc_bundle_js_file")
7878
.and_return(rsc_bundle_name)
7979
allow(ReactOnRails).to receive_message_chain("configuration.server_bundle_output_path")
80-
.and_return(nil)
80+
.and_return("ssr-generated")
8181
end
8282

8383
def mock_dev_server_running
@@ -278,23 +278,21 @@ def mock_dev_server_running
278278
expect(path).to end_with("ssr-generated/#{server_bundle_name}")
279279
end
280280

281-
context "with bundle file existing in standard location but not environment-specific location" do
282-
it "returns the standard location path" do
281+
context "with bundle file existing in ssr-generated location" do
282+
it "returns the ssr-generated location path" do
283283
server_bundle_name = "server-bundle.js"
284284
mock_bundle_configs(server_bundle_name: server_bundle_name)
285285
mock_missing_manifest_entry(server_bundle_name)
286286

287-
# Mock File.exist? to return false for environment-specific path but true for standard path
288-
standard_path = File.expand_path(File.join("public", "packs", server_bundle_name))
289-
env_specific_path = File.join(packer_public_output_path, server_bundle_name)
287+
# Mock File.exist? to return true for ssr-generated path
288+
ssr_generated_path = File.expand_path(File.join("ssr-generated", server_bundle_name))
290289

291290
allow(File).to receive(:exist?).and_call_original
292-
allow(File).to receive(:exist?).with(File.expand_path(env_specific_path)).and_return(false)
293-
allow(File).to receive(:exist?).with(standard_path).and_return(true)
291+
allow(File).to receive(:exist?).with(ssr_generated_path).and_return(true)
294292

295293
path = described_class.server_bundle_js_file_path
296294

297-
expect(path).to eq(standard_path)
295+
expect(path).to eq(ssr_generated_path)
298296
end
299297
end
300298

0 commit comments

Comments
 (0)