Skip to content
35 changes: 32 additions & 3 deletions lib/percy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,25 @@ def self.create_region(
region
end

# Recursively convert all Hash keys (at every nesting level) to symbols so
# config (string keys from JSON) and per-call options (symbol keys) merge on
# consistent keys. Arrays are walked; scalars are returned as-is.
def self.deep_symbolize(obj)
case obj
when Hash then obj.each_with_object({}) { |(k, v), h| h[k.to_sym] = deep_symbolize(v) }
when Array then obj.map { |e| deep_symbolize(e) }
else obj
end
end

# Deep-merge `override` onto `base`: nested Hashes merge recursively, while
# arrays and scalars from `override` replace those in `base`.
def self.deep_merge_options(base, override)
base.merge(override) do |_key, old_val, new_val|
old_val.is_a?(Hash) && new_val.is_a?(Hash) ? deep_merge_options(old_val, new_val) : new_val
end
end

def self.snapshot(driver, name, options = {})
return unless percy_enabled?

Expand All @@ -79,10 +98,20 @@ def self.snapshot(driver, name, options = {})
begin
percy_dom_script = fetch_percy_dom
driver.execute_script(percy_dom_script)
dom_snapshot = if responsive_snapshot_capture?(options)
capture_responsive_dom(driver, options, percy_dom_script: percy_dom_script)

# Merge .percy.yml config options with snapshot options (snapshot options take priority)
config_options = @cli_config&.dig('snapshot') || {}
# Config keys are strings (JSON parse); per-call options use symbols, as
# do downstream consumers (responsive_snapshot_capture?, capture_responsive_dom).
# Deep-symbolize both sides so nested keys are consistent, then deep-merge
# so nested Hashes merge recursively (per-call wins at leaves; arrays/scalars
# replace) instead of a shallow top-level overwrite dropping config siblings.
merged_options = deep_merge_options(deep_symbolize(config_options), deep_symbolize(options))

dom_snapshot = if responsive_snapshot_capture?(merged_options)
capture_responsive_dom(driver, merged_options, percy_dom_script: percy_dom_script)
else
get_serialized_dom(driver, options, percy_dom_script: percy_dom_script)
get_serialized_dom(driver, merged_options, percy_dom_script: percy_dom_script)
end

# Strip `readiness` before POSTing -- SDK-local config that the CLI
Expand Down
79 changes: 79 additions & 0 deletions spec/lib/percy/percy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,85 @@

expect(data).to eq('sync_data')
end

it 'merges .percy.yml config with per-snapshot options (per-call wins)' do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] Test gap: config-only path and POST body unverified

This test covers "config sets two keys, per-call overrides one" but not the motivating case where options come solely from .percy.yml (per-call options = {}). It also does not assert the POST /percy/snapshot body. The POST is built from raw options (post_options = options.reject {...}), not merged_options, so config-only keys reach PercyDOM.serialize but are not re-sent on POST. That asymmetry matches the existing healthcheck pattern but is currently untested.

Suggestion: Add a test with empty per-call options and a populated config snapshot block, asserting config keys reach serialize; optionally add a have_requested(:post, ...) assertion documenting the POST-body contract for config-derived keys.

Reviewer: stack:code-review

# Healthcheck returns a config whose `snapshot` block carries a
# config-only key (enableJavaScript) and a percyCSS value that the
# per-snapshot call will override.
stub_request(:get, "#{Percy::PERCY_SERVER_ADDRESS}/percy/healthcheck")
.to_return(
status: 200,
body: {
success: true,
config: {'snapshot' => {'enableJavaScript' => true, 'percyCSS' => 'FROM_CONFIG'}},
}.to_json,
headers: {'x-percy-core-version': '1.0.0'},
)

stub_request(:get, "#{Percy::PERCY_SERVER_ADDRESS}/percy/dom.js")
.to_return(status: 200, body: fetch_script_string, headers: {})

stub_request(:post, 'http://localhost:5338/percy/snapshot')
.to_return(status: 200, body: '{"success":true}', headers: {})

# Capture the argument passed to PercyDOM.serialize so we can assert how
# config and per-call options were merged before serialization.
captured_serialize_call = nil
allow(page).to receive(:execute_script).and_wrap_original do |original, script, *args|
captured_serialize_call = script if script.to_s.include?('PercyDOM.serialize')
original.call(script, *args)
end

visit 'index.html'
Percy.snapshot(page, 'Name', percyCSS: 'FROM_CALL')

expect(captured_serialize_call).to_not be_nil
serialized = JSON.parse(captured_serialize_call[/PercyDOM\.serialize\((.*)\)/m, 1])
# Config-only key still reaches serialize...
expect(serialized['enableJavaScript']).to eq(true)
# ...and the per-call option wins over the config value.
expect(serialized['percyCSS']).to eq('FROM_CALL')
end

it 'deep-merges nested config and per-snapshot options (sibling kept, leaf overridden)' do
# Config `snapshot` block carries a nested `discovery` hash; the per-call
# discovery only overrides one leaf, so the sibling key must survive.
stub_request(:get, "#{Percy::PERCY_SERVER_ADDRESS}/percy/healthcheck")
.to_return(
status: 200,
body: {
success: true,
config: {
'snapshot' => {
'discovery' => {'networkIdleTimeout' => 50, 'disableCache' => false},
},
},
}.to_json,
headers: {'x-percy-core-version': '1.0.0'},
)

stub_request(:get, "#{Percy::PERCY_SERVER_ADDRESS}/percy/dom.js")
.to_return(status: 200, body: fetch_script_string, headers: {})

stub_request(:post, 'http://localhost:5338/percy/snapshot')
.to_return(status: 200, body: '{"success":true}', headers: {})

captured_serialize_call = nil
allow(page).to receive(:execute_script).and_wrap_original do |original, script, *args|
captured_serialize_call = script if script.to_s.include?('PercyDOM.serialize')
original.call(script, *args)
end

visit 'index.html'
Percy.snapshot(page, 'Name', discovery: {disableCache: true})

expect(captured_serialize_call).to_not be_nil
serialized = JSON.parse(captured_serialize_call[/PercyDOM\.serialize\((.*)\)/m, 1])
# Sibling from config survives; per-call leaf overrides the config value.
expect(serialized['discovery']).to eq(
'networkIdleTimeout' => 50, 'disableCache' => true,
)
end
end
end

Expand Down
21 changes: 19 additions & 2 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,25 @@
Kernel.srand config.seed

# See https://github.com/teamcapybara/capybara#selecting-the-driver for other options
Capybara.default_driver = :selenium_headless
Capybara.javascript_driver = :selenium_headless
# Default to Firefox headless (matches CI), but when a Chromium/Chrome binary is
# provided via CHROME_BIN (e.g. the containerised e2e image), register and use a
# headless Chrome driver pointing at it instead.
if ENV['CHROME_BIN'] && !ENV['CHROME_BIN'].empty?
Capybara.register_driver :selenium_chrome_headless_bin do |app|
options = Selenium::WebDriver::Chrome::Options.new
options.binary = ENV['CHROME_BIN']
options.add_argument('--headless=new')
options.add_argument('--no-sandbox')
options.add_argument('--disable-gpu')
options.add_argument('--disable-dev-shm-usage')
Capybara::Selenium::Driver.new(app, browser: :chrome, options: options)
end
Capybara.default_driver = :selenium_chrome_headless_bin
Capybara.javascript_driver = :selenium_chrome_headless_bin
else
Capybara.default_driver = :selenium_headless
Capybara.javascript_driver = :selenium_headless
end

# Capybara's built-in :selenium_headless driver still passes `options:` to
# driver init, which newer selenium-webdriver logs as a [DEPRECATION]
Expand Down
Loading