diff --git a/Percy.Test/Percy.Test.cs b/Percy.Test/Percy.Test.cs index 3c0d360..b5083ef 100644 --- a/Percy.Test/Percy.Test.cs +++ b/Percy.Test/Percy.Test.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using System.Net.Http; +using System.Reflection; using System.Threading.Tasks; using System.Collections.Generic; using System.Text; @@ -385,6 +386,106 @@ public void PostsSnapshotWithReadinessDisabled() Assert.Contains("Received snapshot: readiness-disabled", logs); } } + + // Verifies the .percy.yml config (cliConfig.snapshot) <-> per-snapshot + // option merge that Percy.Snapshot performs (MergeSnapshotOptions) before + // handing the result to PercyDOM.serialize. Two precedence rules: + // + // - config seeds the merge: a config-only key (enableJavaScript) is + // inherited into the merged options, AND + // - per-call overlays config: a key present in BOTH config and the + // per-snapshot call (percyCSS) is won by the per-snapshot value. + // + // MergeSnapshotOptions is the single source of truth for what reaches + // PercyDOM.serialize, and Selenium 4's WebDriver is not mockable (no public + // parameterless ctor, the ctor starts a real session, and ExecuteScript / + // Manage / Url are non-virtual). So we exercise the merge directly through + // its private static surface via reflection — fully deterministic, no + // browser and no CLI server required. + public class MergePrecedenceTests + { + private static Dictionary InvokeMerge( + JsonElement cliConfig, Dictionary? perSnapshotOptions) + { + // Seed the SDK's cliConfig (internal setter, visible to Percy.Test). + Percy.ResetInternalCaches(); + Percy.setCliConfig(cliConfig); + + MethodInfo merge = typeof(Percy).GetMethod( + "MergeSnapshotOptions", BindingFlags.NonPublic | BindingFlags.Static)!; + object? result = merge.Invoke(null, new object?[] { perSnapshotOptions }); + return (Dictionary)result!; + } + + [Fact] + public void PerSnapshotOptionWinsOverConfigDuringMerge() + { + try + { + // .percy.yml config: enableJavaScript is config-only; percyCSS is + // set here AND overridden per-snapshot to exercise the conflict. + JsonElement cliConfig = JsonSerializer.Deserialize( + "{\"snapshot\":{\"enableJavaScript\":true,\"percyCSS\":\"FROM_CONFIG\"}}"); + + var perSnapshotOptions = new Dictionary + { + { "percyCSS", "FROM_CALL" } + }; + + Dictionary merged = InvokeMerge(cliConfig, perSnapshotOptions); + + // Config seeds the merge: the config-only key is inherited. + Assert.True(merged.ContainsKey("enableJavaScript")); + Assert.Equal(true, merged["enableJavaScript"]); + + // Per-call overlays config: the per-snapshot value wins on conflict. + Assert.Equal("FROM_CALL", merged["percyCSS"]); + } + finally + { + Percy.ResetInternalCaches(); + } + } + + [Fact] + public void NestedConfigObjectIsDeepMergedWithPerSnapshotOptions() + { + try + { + // .percy.yml config: nested discovery object with two leaves. + JsonElement cliConfig = JsonSerializer.Deserialize( + "{\"snapshot\":{\"discovery\":{\"networkIdleTimeout\":50,\"disableCache\":false}}}"); + + // Per-snapshot call overrides only one leaf of the nested object. + var perSnapshotOptions = new Dictionary + { + { + "discovery", new Dictionary + { + { "disableCache", true } + } + } + }; + + Dictionary merged = InvokeMerge(cliConfig, perSnapshotOptions); + + // Nested object is deep-merged, not replaced wholesale. + Assert.True(merged.ContainsKey("discovery")); + var discovery = Assert.IsType>(merged["discovery"]); + + // Config-only leaf is preserved. + Assert.Equal(50, discovery["networkIdleTimeout"]); + + // Per-call leaf wins at the leaf level. + Assert.Equal(true, discovery["disableCache"]); + } + finally + { + Percy.ResetInternalCaches(); + } + } + } + public class RegionTests { [Fact] diff --git a/Percy.Test/PercyLogic.Test.cs b/Percy.Test/PercyLogic.Test.cs index 3a82816..6a3d6bf 100644 --- a/Percy.Test/PercyLogic.Test.cs +++ b/Percy.Test/PercyLogic.Test.cs @@ -188,6 +188,75 @@ public void JsonElementToObject_MapsComplexKindsToRawText() Assert.Contains("\"a\"", obj); } + // ===== JsonElementToObjectDeep / MergeSnapshotOptions (PER-8053) ======= + + [Fact] + public void JsonElementToObjectDeep_CoversAllValueKinds() + { + // Recursive converter behind config-merge; exercise every ValueKind branch. + Assert.IsType>( + InvokePrivate("JsonElementToObjectDeep", ParseJson("{\"a\":1}"))!); + // all-integer arrays collapse to List (e.g. widths) + Assert.IsType>( + InvokePrivate("JsonElementToObjectDeep", ParseJson("[375,1280]"))!); + // mixed arrays stay List + Assert.IsType>( + InvokePrivate("JsonElementToObjectDeep", ParseJson("[1,\"x\"]"))!); + Assert.Equal(true, InvokePrivate("JsonElementToObjectDeep", ParseJson("true"))); + Assert.Equal(false, InvokePrivate("JsonElementToObjectDeep", ParseJson("false"))); + Assert.Equal(7, InvokePrivate("JsonElementToObjectDeep", ParseJson("7"))); + Assert.Equal(1.5, InvokePrivate("JsonElementToObjectDeep", ParseJson("1.5"))); + Assert.Equal("s", InvokePrivate("JsonElementToObjectDeep", ParseJson("\"s\""))); + Assert.Null(InvokePrivate("JsonElementToObjectDeep", ParseJson("null"))); + } + + [Fact] + public void MergeSnapshotOptions_MergesCliConfigWithPerCallOptions() + { + // Global .percy.yml snapshot config merged with per-call options; per-call wins. + SetCliConfigJson("{\"snapshot\":{\"enableJavaScript\":true,\"widths\":[375,1280],\"percyCSS\":\"FROM_CONFIG\"}}"); + var options = new Dictionary { { "percyCSS", "FROM_CALL" } }; + var merged = (Dictionary)InvokePrivate("MergeSnapshotOptions", options)!; + Assert.True((bool)merged["enableJavaScript"]); // inherited from config + Assert.IsType>(merged["widths"]); // deep-converted from config + Assert.Equal("FROM_CALL", merged["percyCSS"]); // per-call overrides config + } + + [Fact] + public void MergeSnapshotOptions_NullOptionsAndNoConfig_ReturnsEmpty() + { + // Covers the null-options branch and the no-cli-config branch. + var merged = (Dictionary)InvokePrivate("MergeSnapshotOptions", new object?[] { null })!; + Assert.NotNull(merged); + } + + [Fact] + public void MergeSnapshotOptions_DeepMergesNestedObjects() + { + // Nested objects present in both config and per-call options merge + // recursively (exercises the DeepMerge recursion branch). + SetCliConfigJson("{\"snapshot\":{\"discovery\":{\"networkIdleTimeout\":100,\"disableCache\":true}}}"); + var options = new Dictionary + { + { "discovery", new Dictionary { { "networkIdleTimeout", 200 } } } + }; + var merged = (Dictionary)InvokePrivate("MergeSnapshotOptions", options)!; + var discovery = (Dictionary)merged["discovery"]; + Assert.Equal(200, discovery["networkIdleTimeout"]); // per-call wins at the leaf + Assert.Equal(true, discovery["disableCache"]); // config leaf inherited + } + + [Fact] + public void MergeSnapshotOptions_NonObjectCliConfig_SwallowsAndReturnsOptions() + { + // A non-object cliConfig makes TryGetProperty("snapshot") throw; the + // catch swallows it and per-call options still flow through. + SetCliConfigJson("[1,2,3]"); + var options = new Dictionary { { "percyCSS", "x" } }; + var merged = (Dictionary)InvokePrivate("MergeSnapshotOptions", options)!; + Assert.Equal("x", merged["percyCSS"]); + } + // ===== ResolveReadinessConfig ========================================= [Fact] diff --git a/Percy/Percy.cs b/Percy/Percy.cs index 8dda0b5..e47baf6 100644 --- a/Percy/Percy.cs +++ b/Percy/Percy.cs @@ -1476,6 +1476,106 @@ public static List> CaptureResponsiveDom(WebDriver dr return domSnapshots; } + private static Dictionary MergeSnapshotOptions(Dictionary? options) + { + var merged = new Dictionary(); + if (cliConfig != null) + { + try + { + JsonElement config = (JsonElement)cliConfig; + if (config.TryGetProperty("snapshot", out JsonElement snapshotElement) && + snapshotElement.ValueKind == JsonValueKind.Object) + { + foreach (JsonProperty prop in snapshotElement.EnumerateObject()) + { + // Recursively convert config values so nested JSON + // objects become Dictionary and can be + // deep-merged with per-call options below. + var converted = JsonElementToObjectDeep(prop.Value); + if (converted != null) + merged[prop.Name] = converted; + } + } + } + catch (Exception) { } + } + if (options != null) + { + // Deep-merge: nested objects merge recursively (per-call wins at + // leaves), arrays/scalars replace. Matches the JS sdk-utils fix. + merged = DeepMerge(merged, options); + } + return merged; + } + + // Recursively converts a JSON element into plain CLR objects: + // Object -> Dictionary (recursive), + // Array -> List (recursive), + // primitives -> bool / int / double / string. + private static object? JsonElementToObjectDeep(JsonElement el) + { + switch (el.ValueKind) + { + case JsonValueKind.Object: + var dict = new Dictionary(); + foreach (JsonProperty prop in el.EnumerateObject()) + { + var converted = JsonElementToObjectDeep(prop.Value); + if (converted != null) + dict[prop.Name] = converted; + } + return dict; + case JsonValueKind.Array: + var items = el.EnumerateArray().ToList(); + // All-integer arrays (e.g. config `widths`) must stay List so + // downstream consumers like CaptureResponsiveDom can use them directly. + if (items.Count > 0 && items.All(i => i.ValueKind == JsonValueKind.Number && i.TryGetInt32(out _))) + return items.Select(i => i.GetInt32()).ToList(); + var list = new List(); + foreach (JsonElement item in items) + { + var converted = JsonElementToObjectDeep(item); + if (converted != null) + list.Add(converted); + } + return list; + case JsonValueKind.True: + return true; + case JsonValueKind.False: + return false; + case JsonValueKind.Number: + return el.TryGetInt32(out int intVal) ? (object)intVal : el.GetDouble(); + case JsonValueKind.String: + return el.GetString(); + default: + return null; + } + } + + // Deep-merges override into a copy of baseDict: when a key exists in both + // and both values are Dictionary, recurse; otherwise the + // override value wins (arrays and scalars replace). + private static Dictionary DeepMerge( + Dictionary baseDict, Dictionary overrideDict) + { + var result = new Dictionary(baseDict); + foreach (var kvp in overrideDict) + { + if (result.TryGetValue(kvp.Key, out var existing) && + existing is Dictionary existingDict && + kvp.Value is Dictionary overrideNested) + { + result[kvp.Key] = DeepMerge(existingDict, overrideNested); + } + else + { + result[kvp.Key] = kvp.Value; + } + } + return result; + } + private static bool isResponsiveSnapshotCapture(Dictionary? options) { if (cliConfig == null) return false; @@ -1531,14 +1631,16 @@ public class Options : Dictionary {} // Non-Chrome browsers and missing ExecuteCdpCommand are no-ops. ExposeClosedShadowRoots(driver); + // Merge .percy.yml config options with snapshot options (snapshot options take priority) + var mergedOptions = MergeSnapshotOptions(options); + var cookies = driver.Manage().Cookies.AllCookies; - string opts = JsonSerializer.Serialize(options); dynamic domSnapshot = null; - if (isResponsiveSnapshotCapture(options)) { - domSnapshot = CaptureResponsiveDom(driver, cookies, options); + if (isResponsiveSnapshotCapture(mergedOptions)) { + domSnapshot = CaptureResponsiveDom(driver, cookies, mergedOptions); } else { - domSnapshot = getSerializedDom(driver, cookies, options, _dom); + domSnapshot = getSerializedDom(driver, cookies, mergedOptions, _dom); } Options snapshotOptions = new Options {