feat: CORS iframe + closed shadow DOM parity#436
Merged
Conversation
…ore, and recovery controls Replace the single-level processFrame helper with a recursive ProcessFrameTree pipeline that mirrors the canonical Percy CORS iframe spec from percy/percy-nightwatch#869. Adds: - DEFAULT_MAX_FRAME_DEPTH/MAX_ALLOWED_FRAME_DEPTH, ClampFrameDepth, NormalizeIgnoreSelectors, ResolveMaxFrameDepth, ResolveIgnoreSelectors inlined since .NET has no @percy/sdk-utils equivalent. - ENUMERATE_IFRAMES_SCRIPT + IframeInfo to collect iframe metadata in one round-trip per frame context. - ShouldSkipIframe centralizes filtering: data-percy-ignore attribute, ignoreIframeSelectors matches, unsupported / srcdoc / same-origin frames, and frames without data-percy-element-id are dropped early. - ProcessFrameTree recurses cross-origin descendants up to MaxFrameDepth, with a HashSet ancestor-URL chain to break cyclic A->B->A graphs. - Post-switch URL re-check: after SwitchTo().Frame, the loaded document.URL is rechecked against IsUnsupportedIframeSrc to handle late navigations. - PercyContextLostException carries the partial capture so a failed SwitchTo().ParentFrame() unwind still surfaces every frame serialized up to the failure. CaptureCorsIframes wires the helpers into getSerializedDom, replacing the prior ad-hoc top-level iframe loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors percy/percy-playwright#609. Walks the CDP DOM tree (DOM.getDocument with depth=-1 and pierce=true), collects backendNodeId pairs for every closed shadow root, resolves both sides via DOM.resolveNode, then uses Runtime.callFunctionOn to register each shadow root in a window-bound WeakMap (window.__percyClosedShadowRoots) that PercyDOM.serialize() reads during cloning. Nodes inside child frame documents are skipped because their execution contexts don't share the WeakMap. Wired into Snapshot() before serialization and into the responsive capture reload path so the WeakMap survives page.reload(). No-op on non-Chrome drivers and when ExecuteCdpCommand isn't available, so existing Firefox / non-Chrome test paths are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Covers the inlined helpers (GetOrigin, IsUnsupportedIframeSrc,
ClampFrameDepth, NormalizeIgnoreSelectors), the ShouldSkipIframe skip
matrix (data-percy-ignore, ignoreIframeSelectors, unsupported src,
srcdoc, same-origin, missing percyElementId, and immediate-parent
origin comparison), and the PercyContextLostException carrier.
Tests rely on the existing InternalsVisibleTo("Percy.Test") in
AssemblyInfo.cs and use reflection for the private IframeInfo /
ShouldSkipIframe symbols so the production API stays sealed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add volatile to the _http field so the unlocked outer read in getHttpClient sees a fully-published HttpClient (Timeout set) on weak memory models. Without it the DCL pattern provides no happens-before guarantee on ARM/Apple Silicon. - Pair DOM.enable with DOM.disable in a finally block so the CDP session doesn't keep emitting DOM events after every snapshot + resize/reload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two reflection-based unit tests confirm the invariant the newly-volatile _http field exists to protect: - First getHttpClient() call returns a client whose Timeout is already TimeSpan.FromMinutes(10) (the bug we fixed: setting Timeout AFTER assigning the field could let concurrent callers observe a default 100s timeout). - Subsequent calls return the same instance — DCL must not rebuild. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CE re-review flagged ClampFrameDepth silently mapping user-supplied 0 to DEFAULT_MAX_FRAME_DEPTH (5) as a hidden behavior. The intent matches @percy/sdk-utils clampFrameDepth and the JS SDKs: 0 means "use default" (unset / config placeholder), not "disable iframe capture". To disable nested capture, users should omit the option or use data-percy-ignore / ignoreIframeSelectors. No behavior change — pure documentation so future readers don't re-flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CE re-review flagged that CollectClosedShadowRoots early-returned on any
node carrying contentDocument, so closed shadow roots living inside
same-origin iframes were silently skipped — even though those frames
share the parent's JS realm and therefore the same
window.__percyClosedShadowRoots WeakMap that PercyDOM.serialize reads.
Walker now decides per-frame:
* Same origin (compared via GetOrigin against the page URL) -> recurse
INTO contentDocument; found closed roots use the parent realm's map.
* Cross origin -> still skipped (different JS realm, the resolveNode
objectIds don't belong to our execution context anyway).
* Missing documentURL / unknown origin -> defensive skip (pre-fix
behavior for the unknown case).
Also factored the CDP flow inside ExposeClosedShadowRoots into a pure
internal RunClosedShadowRootExposure that takes a CDP invoker delegate,
a script-runner delegate, and a page-URL getter. The WebDriver-bound
entrypoint now adapts the reflected ExecuteCdpCommand to those delegates,
which makes the DOM.enable / DOM.disable lifecycle directly unit-testable
without a real browser.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses CE re-review MINORs and MAJOR coverage gaps:
* Same-origin closed-shadow-in-iframe recursion: three new tests drive
CollectClosedShadowRoots with synthesized CDP DOM payloads — one
same-origin iframe (closed root inside contentDocument is captured),
one cross-origin iframe (skipped), one with missing documentURL
(defensive skip). Plus a sanity test that a top-level closed root
still works.
* DOM.disable invocation regression coverage: a FakeCdp recorder
exercises RunClosedShadowRootExposure end-to-end and asserts:
- DOM.disable is sent after a successful run, as the final command.
- DOM.disable is still sent if DOM.getDocument throws.
- DOM.disable is NOT sent when DOM.enable itself throws (domEnabled
stays false in the finally block — avoids spurious disable on a
session that never enabled the domain).
* HttpClient state serialization: the GetHttpClient_* tests mutate the
static volatile _http field. Tag the suite with
[Collection("HttpClientStateSerial")] (DisableParallelization = true)
so a future flip of parallelizeTestCollections in xunit.runner.json
can't race them against Percy.Test.cs / PercyDriver.Test.cs, which
also touch setHttpClient.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nd-shadow-dom # Conflicts: # package.json
pranavz28
reviewed
May 28, 2026
pranavz28
left a comment
pranavz28
left a comment
Contributor
There was a problem hiding this comment.
Cross-SDK review across the 4 CORS-iframe PRs. The .NET port mirrors the canonical Nightwatch implementation #869.
Verified-pass items
getHttpClient(Percy.cs:102-114): DCL withvolatile,client.Timeoutset BEFORE_http = client. Correct under ARM/Apple Silicon weak memory.DOM.enable/DOM.disablelifecycle gated bydomEnabledsodisableis suppressed whenenableitself throws. Unit-tested across all three paths.- Same-origin closed-shadow-in-iframe recursion now captures inner roots via the parent realm's WeakMap; cross-origin still skipped via
GetOrigin. Defensive skip on missingdocumentURL. ExecuteCdpCommandvia reflection: null-checked, logs debug, returns gracefully on non-Chrome.- Cycle detection: HashSet seeded with both
info.SrcAND post-switchframeUrlfor children — correct, and stronger than the webdriverio port. PercyContextLostExceptioncarriesPartialCaptureupward through nested rethrow; outerCaptureCorsIframesdrains the payload on catch.ClampFrameDepth(0) = defaultis documented and consistent across SDKs.
Additional finding (no precise line anchor)
IsUnsupportedIframeSrc test theory is incomplete — Percy.Test/CorsIframesTest.cs Theory rows for IsUnsupportedIframeSrc only validate the 3 schemes the impl already handles. No row asserts about:blank or blob:https://... => true. Once you add the missing prefixes (see inline on Percy.cs:353), extend the theory rows so we don't silently regress.
Verdict: request changes. Fix the inline scheme-list gap + tighten the test Collection coverage before merge.
| return string.IsNullOrEmpty(src) || | ||
| src.StartsWith("javascript:", StringComparison.OrdinalIgnoreCase) || | ||
| src.StartsWith("data:", StringComparison.OrdinalIgnoreCase) || | ||
| src.StartsWith("vbscript:", StringComparison.OrdinalIgnoreCase); |
Contributor
There was a problem hiding this comment.
HIGH — IsUnsupportedIframeSrc is missing about:, blob:, and file: prefixes.
The canonical Nightwatch port (#869) and the comment at Percy.cs:651 ("about:blank, javascript:, data:, ...") both expect the full list. Today this only checks javascript:, data:, vbscript: plus null/empty.
Practical impact:
about:blanksurvives the post-switch re-check via incidental behavior —GetOrigin("about:blank")returns empty, so it eventually falls into the same-origin/invalid-origin skip. That's accidental, not by design.blob:https://example.com/...slips through both the pre- and post-switch checks becauseGetOriginsucceeds and returns a real origin. The SDK will thenSwitchTo().Frame(...)into a blob frame and try to serialize it.
Fix:
return string.IsNullOrEmpty(src) ||
src.StartsWith("about:", StringComparison.OrdinalIgnoreCase) ||
src.StartsWith("javascript:", StringComparison.OrdinalIgnoreCase) ||
src.StartsWith("data:", StringComparison.OrdinalIgnoreCase) ||
src.StartsWith("blob:", StringComparison.OrdinalIgnoreCase) ||
src.StartsWith("file:", StringComparison.OrdinalIgnoreCase) ||
src.StartsWith("vbscript:", StringComparison.OrdinalIgnoreCase);Also extend the matching Theory in CorsIframesTest.cs so each of the four ports stays aligned.
Contributor
Author
There was a problem hiding this comment.
Fixed in 67bbb5e. IsUnsupportedIframeSrc now uses the full canonical list (about:, chrome:, chrome-extension:, devtools:, edge:, opera:, view-source:, data:, javascript:, blob:, vbscript:, file:) matching the nightwatch/protractor ports. CorsIframesTest.cs adds theory rows for about:blank, blob:, file: (incl. case-insensitivity), chrome:, and view-source:.
| // Unit tests for the CORS iframe + closed shadow DOM helpers added to | ||
| // Percy.cs. These don't require the Percy CLI or a real browser; they | ||
| // exercise the pure-C# helpers via reflection where they are internal. | ||
| [Collection("HttpClientStateSerial")] |
Contributor
There was a problem hiding this comment.
MEDIUM — [Collection("HttpClientStateSerial")] is applied here but not on the other test classes that mutate the same static state.
Percy.Test/Percy.Test.cs:29 and Percy.Test/PercyDriver.Test.cs:65,99,137 also call Percy.setHttpClient(...) and would race against this class if parallelizeTestCollections is ever flipped to true. The comment block above this line says the attribute "pins the invariant" — but the invariant only holds if every static-state-mutating class joins the same collection.
Options:
- Tag
Percy.Test.csandPercyDriver.Test.cswith the same[Collection("HttpClientStateSerial")](preferred). - Or soften the comment to acknowledge the gap so a future reader doesn't trust it as enforced.
Contributor
Author
There was a problem hiding this comment.
Fixed in 67bbb5e. Added [Collection("HttpClientStateSerial")] to both UnitTests (Percy.Test.cs) and PercyDriverTest (PercyDriver.Test.cs). Both classes touch Percy.setHttpClient / the static _http field, so the collection now correctly serializes every suite that mutates that shared state.
- Percy.cs: expand IsUnsupportedIframeSrc to the canonical list (about:, chrome:, chrome-extension:, devtools:, edge:, opera:, view-source:, data:, javascript:, blob:, vbscript:, file:), matching the nightwatch/protractor ports. Blocks about:blank, blob:, and file:// iframes that previously slipped through. - Percy.Test.cs / PercyDriver.Test.cs: opt UnitTests and PercyDriverTest into the HttpClientStateSerial collection so GetHttpClient_* tests no longer race them on Percy._http. - CorsIframesTest.cs: cover the new prefixes and case-insensitivity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pranavz28
approved these changes
May 29, 2026
pranavz28
left a comment
pranavz28
left a comment
Contributor
There was a problem hiding this comment.
Check once if the package.json is updated according to the lock file
Master's lockfile was stale (pinned @percy/cli 1.31.10-beta.2 while package.json declared ^1.31.15-beta.0), and the previous regeneration on this branch was done with an npm version that stripped the "license" fields — producing ~124 lines of phantom diff. Regenerated with npm 10.8.2 so license fields are preserved and the lockfile resolves the version package.json actually asks for. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This PR is stale because it has been open for more than 14 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
AkashBrowserStack
added a commit
that referenced
this pull request
Jun 22, 2026
The master merge (#436) replaced the single-level CORS-iframe implementation (driver.FindElements(By.TagName("iframe")) + SwitchTo().Frame, with a "Fatal" message rethrow bubbling to Snapshot's outer catch) with the recursive ENUMERATE_IFRAMES_SCRIPT / ProcessFrameTree pipeline that: - enumerates iframes via ExecuteScript(querySelectorAll('iframe')), not FindElements; - resolves each frame element by data-percy-element-id via querySelector; - switches in with SwitchTo().Frame and restores with SwitchTo().ParentFrame (DefaultContent fallback), raising PercyContextLostException on a lost nested context instead of rethrowing on a "Fatal" substring. Three coverage-gate tests still asserted the old command surface and so failed on the net10/Linux CI runner (Assert "findElements"/"switchToFrame" not found; empty Snapshot error output). Rewritten to drive the real frame-tree path deterministically through the FakeWebDriver seam: - Snapshot_ProcessesCrossOriginIframe: asserts querySelectorAll enumeration + SwitchToFrame/SwitchToParentFrame. - Snapshot_IframeSerializeThrows_IsCaughtAndSkipped: serialize-in-frame throws -> non-fatal skip, ParentFrame restore still runs, snapshot still posts. - Snapshot_FatalFrameError_RethrowsThroughBothCatches -> Snapshot_NestedFrameLosesParentContext_AbortsAndShipsPartial: depth-2 ParentFrame failure raises PercyContextLostException; CaptureCorsIframes aborts further capture, merges partials, Snapshot does not throw. Also points PercyLogicTest at the surviving HttpClientStateSerial collection (PercyLogicSerial was removed by the merge consolidating all static-state classes into one non-parallel collection). No production code changed; gate stays /p:ThresholdType=line /p:Threshold=100. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AkashBrowserStack
added a commit
that referenced
this pull request
Jun 22, 2026
After merging master (#436 "CORS iframe + closed shadow DOM parity"), Percy.cs gained a large new driver-integration surface (EnumerateIframes, recursive ProcessFrameTree with depth/cycle/context-loss handling, CaptureCorsIframes, ResolveMaxFrameDepth/ResolveIgnoreSelectors, ExposeClosedShadowRoots + RunClosedShadowRootExposure CDP pipeline, CollectClosedShadowRoots). Master's own CorsIframesTest covered the pure-logic helpers, leaving the driver-driven paths uncovered (merged tree was 81.59% line). This adds real tests for all of them via the in-process FakeWebDriver / FakeChromeWebDriver seams, taking the merged Percy.cs to a genuine 100% line coverage (gate Threshold=100, exit 0). Added (Percy.Test/CorsIframesTest.cs, +~42 tests): - ResolveMaxFrameDepth / ResolveIgnoreSelectors: options + cliConfig (string & array) + wrong-type catch fall-throughs + defaults. - EnumerateIframes: dictionary coercion and non-enumerable -> empty. - ProcessFrameTree: happy-path capture+recurse, depth cap, cyclic-ancestor skip, element null / resolve-throws, post-switch unsupported-URL skip, URL-read-throws fallback-to-src, serialize throws/null, nested cross-origin child roll-up, generic post-switch catch, ParentFrame failure at depth 1 (swallowed) and depth 2 (PercyContextLostException with partial capture). - CaptureCorsIframes: no-iframes, top-level cross-origin captured, enumerate throws (outer catch), context-lost-from-child aborts with partial. - ExposeClosedShadowRoots: non-Chrome no-op, Chrome-without-CDP logs+returns, Chrome-with-CDP drives pipeline, pageUrlGetter-throws swallowed. - RunClosedShadowRootExposure: full resolveNode->objectId->callFunctionOn happy path, missing-objectId skip, per-pair resolve-throws catch, getDocument-null / root-missing early returns, DOM.disable-throws-in-finally swallowed. - CollectClosedShadowRoots: contentDocument-not-a-dict, non-closed shadow root, shadow-item-not-a-dict, non-dict node. TryGetLong & ExtractObjectId all branches. Percy.Test/FakeWebDriver.cs: FakeChromeWebDriver gains an optional pluggable CdpResult delegate so closed-shadow tests can return real DOM.getDocument / DOM.resolveNode / Runtime.callFunctionOn payloads; default behavior unchanged. Percy/Percy.cs (one removal, behavior-preserving): EnumerateIframes had an `else if (item is Dictionary<string, object>)` fallback after `if (item is IDictionary<string, object?>)`. Nullable reference annotations are erased at runtime, so IDictionary<string,object?> and IDictionary<string,object> are the SAME runtime type — every Dictionary<string,object> the in-browser enumerate script can return always matches the first arm, making the else-if provably unreachable dead code. Verified empirically (the `is` check is True at runtime). Removed it (replaced with an explanatory comment) so 100% is reached with real tests and zero coverage exclusions/pragmas; the removed path could never execute, so runtime behavior is identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AkashBrowserStack
added a commit
that referenced
this pull request
Jun 24, 2026
) * test: wire coverlet coverage gate + raise non-driver coverage 28% -> 54% The SDK had coverlet present but unwired (no gate). This enables coverlet (adds coverlet.msbuild) and a 53% line gate in `npm test`, and adds a mock/HTTP-based PercyLogicTest (56 tests) covering the non-driver logic. Coverage (non-driver, local): 27.66% -> 54.16% line. The CI run (`npm test` = percy exec + full suite incl. the live-Firefox tests, which pass on the browser CI) covers more; 53% is the honest floor for the deterministic non-driver logic. New tests cover (via RichardSzalay.MockHttp + the internal setCliConfig seam): GetOrigin, IsUnsupportedIframeSrc, JSON helpers, ResolveReadiness Config merge, min-height/responsive-target resolution, isResponsive SnapshotCapture branches, PayloadParser, CreateRegion variants, Enabled healthcheck branches + caching, GetPercyDOM, GetResponsiveWidths, Log. Uncovered = live-Firefox/WebDriver only (Snapshot/getSerializedDom/ ProcessFrame/WaitForReady/CaptureResponsiveDom/ChangeWindowDimension and PercySeleniumDriver's reflection-on-real-driver paths) — exercised by the existing UnitTests class on the browser CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: raise line coverage 54% -> 96% via in-process WebDriver fake; gate floor 91 The driver-bound half of the SDK (Percy.Snapshot web flow, getSerializedDom, ProcessFrame CORS-iframe handling, WaitForReady, CaptureResponsiveDom + ChangeWindowDimension/CDP resize, the Screenshot automate flow, and the entire PercySeleniumDriver/PercyDriver reflection surface) was previously only reached by the live-Firefox UnitTests class on CI. This adds deterministic, in-process coverage for all of it WITHOUT a browser and WITHOUT any production changes. Seam (zero production edits): a concrete FakeWebDriver subclasses Selenium's abstract WebDriver and overrides ONLY the single protected virtual ExecuteAsync(command, parameters) chokepoint that every WebDriver command routes through. This drives the real, sealed public Selenium methods (ExecuteScript, ExecuteAsyncScript, FindElements, SwitchTo, Manage().Window/Cookies/Timeouts, Navigate().Refresh) through genuine Selenium decoding/element-materialisation code. @percy/cli HTTP stays mocked with RichardSzalay.MockHttp. NewSession echoes the requested firstMatch caps so the real Capabilities property — and PercySeleniumDriver's reflection over ReturnedCapabilities/HttpCommandExecutor — exercises the actual production paths. A FakeChromeWebDriver exposes ExecuteCdpCommand to drive the CDP resize branch. Local line coverage (deterministic non-live suite): 54.16% -> 96.29%. PercyDriver, PercySeleniumDriver, Cache and all Region/Options types are now 100%. Remaining uncovered Percy.cs lines are env-gated static-readonly branches (PERCY_LOGLEVEL=debug, PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT/RELOAD_PAGE), a real-ChromeDriver type check, and effectively-dead catch arms. Gate: green floor raised 53 -> 91 (Threshold/p:ThresholdType=line), ~5 points below the locally-achieved 96.29% for macOS<->Linux variance. CI runs the full npm test (live UnitTests included) on pull_request, so CI coverage is >= local. Verified the gate fails the build when coverage drops below 91. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: reach genuine 100% line coverage; raise coverlet gate 91 -> 100 Closes the remaining ~3.7% (96.29% -> 100.00% line) with real tests over the real branches; no coverage exclusions, pragmas, or [ExcludeFromCodeCoverage]. Behavior-preserving seams in Percy/Percy.cs (production runtime identical): - internal static mirrors of the env-derived static-readonly flags (DebugEnabled, ResponsiveCaptureMinHeight, ResponsiveCaptureReloadPage, ResponsiveCaptureSleepTime), each initialized to the exact value of its public readonly source field; internal read-sites now read the mirror. Tests flip them (via InternalsVisibleTo) to cover both branches without mutating process env. Mirrors the percy-playwright-dotnet pattern. - internal static MinHeightParser seam (defaults to the original int.TryParse logic) so the otherwise-unreachable FormatException catch in ResolveConfiguredMinHeight can be driven by a test. - ResetInternalCaches restores all mirrors + the parse seam to env defaults so per-test overrides cannot leak. New tests cover: Log DEBUG-true catch+finally arms; ResolveResponsiveTargetHeight minHeight-enabled paths; the FormatException catch; IsChromeBrowser's `driver is ChromeDriver` true branch (real ChromeDriver type via GetUninitializedObject, no browser launch); responsive reload-page + sleep-time branches; and the inner+outer Fatal-frame rethrows in getSerializedDom. Gate: package.json `dotnet test ... /p:ThresholdType=line /p:Threshold=100`. Verified locally: 132 tests pass, coverlet reports 100% line (line-rate 1.0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: force serial execution to fix static-state races on CI The 100%-coverage suite passed locally but 3 CORS-iframe driver-flow tests failed deterministically on the Linux CI runner (Snapshot_ProcessesCrossOriginIframe, Snapshot_IframeSerializeThrows_IsCaughtAndSkipped, Snapshot_FatalFrameError). Root cause: the SDK keeps process-wide mutable statics (Percy._http, _dom, _enabled, sessionType, cliConfig). Tests mutate these and rely on them persisting across several driver/HTTP calls within one test. Under parallel execution they race: e.g. one test resets Percy._http to a bare HttpClient (real network) while another is mid-Snapshot, so its GetPercyDOM hits the live `percy --testing` server, gets an empty dom.js body, caches _dom = "" and silently skips iframe processing -> the iframe asserts see no findElements/switchToFrame command. Reproduced locally by flipping xunit.runner.json to parallel: 6-10 nondeterministic failures per run, matching the CI symptom. xunit.runner.json already sets parallelizeTestCollections:false but the net10.0 + xunit.runner.visualstudio 2.4.5 adapter combo did not honor it on the Linux runner (it ran collections in parallel). Adding the authoritative assembly-level [assembly: CollectionBehavior(DisableTestParallelization = true)] forces serial execution when the json directive is ignored. No production code affected; coverage remains 100% line (gate Threshold=100). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: update CORS-iframe coverage tests to the post-#436 frame-tree API The master merge (#436) replaced the single-level CORS-iframe implementation (driver.FindElements(By.TagName("iframe")) + SwitchTo().Frame, with a "Fatal" message rethrow bubbling to Snapshot's outer catch) with the recursive ENUMERATE_IFRAMES_SCRIPT / ProcessFrameTree pipeline that: - enumerates iframes via ExecuteScript(querySelectorAll('iframe')), not FindElements; - resolves each frame element by data-percy-element-id via querySelector; - switches in with SwitchTo().Frame and restores with SwitchTo().ParentFrame (DefaultContent fallback), raising PercyContextLostException on a lost nested context instead of rethrowing on a "Fatal" substring. Three coverage-gate tests still asserted the old command surface and so failed on the net10/Linux CI runner (Assert "findElements"/"switchToFrame" not found; empty Snapshot error output). Rewritten to drive the real frame-tree path deterministically through the FakeWebDriver seam: - Snapshot_ProcessesCrossOriginIframe: asserts querySelectorAll enumeration + SwitchToFrame/SwitchToParentFrame. - Snapshot_IframeSerializeThrows_IsCaughtAndSkipped: serialize-in-frame throws -> non-fatal skip, ParentFrame restore still runs, snapshot still posts. - Snapshot_FatalFrameError_RethrowsThroughBothCatches -> Snapshot_NestedFrameLosesParentContext_AbortsAndShipsPartial: depth-2 ParentFrame failure raises PercyContextLostException; CaptureCorsIframes aborts further capture, merges partials, Snapshot does not throw. Also points PercyLogicTest at the surviving HttpClientStateSerial collection (PercyLogicSerial was removed by the merge consolidating all static-state classes into one non-parallel collection). No production code changed; gate stays /p:ThresholdType=line /p:Threshold=100. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: cover post-#436 CORS-iframe + closed-shadow-DOM pipeline to 100% After merging master (#436 "CORS iframe + closed shadow DOM parity"), Percy.cs gained a large new driver-integration surface (EnumerateIframes, recursive ProcessFrameTree with depth/cycle/context-loss handling, CaptureCorsIframes, ResolveMaxFrameDepth/ResolveIgnoreSelectors, ExposeClosedShadowRoots + RunClosedShadowRootExposure CDP pipeline, CollectClosedShadowRoots). Master's own CorsIframesTest covered the pure-logic helpers, leaving the driver-driven paths uncovered (merged tree was 81.59% line). This adds real tests for all of them via the in-process FakeWebDriver / FakeChromeWebDriver seams, taking the merged Percy.cs to a genuine 100% line coverage (gate Threshold=100, exit 0). Added (Percy.Test/CorsIframesTest.cs, +~42 tests): - ResolveMaxFrameDepth / ResolveIgnoreSelectors: options + cliConfig (string & array) + wrong-type catch fall-throughs + defaults. - EnumerateIframes: dictionary coercion and non-enumerable -> empty. - ProcessFrameTree: happy-path capture+recurse, depth cap, cyclic-ancestor skip, element null / resolve-throws, post-switch unsupported-URL skip, URL-read-throws fallback-to-src, serialize throws/null, nested cross-origin child roll-up, generic post-switch catch, ParentFrame failure at depth 1 (swallowed) and depth 2 (PercyContextLostException with partial capture). - CaptureCorsIframes: no-iframes, top-level cross-origin captured, enumerate throws (outer catch), context-lost-from-child aborts with partial. - ExposeClosedShadowRoots: non-Chrome no-op, Chrome-without-CDP logs+returns, Chrome-with-CDP drives pipeline, pageUrlGetter-throws swallowed. - RunClosedShadowRootExposure: full resolveNode->objectId->callFunctionOn happy path, missing-objectId skip, per-pair resolve-throws catch, getDocument-null / root-missing early returns, DOM.disable-throws-in-finally swallowed. - CollectClosedShadowRoots: contentDocument-not-a-dict, non-closed shadow root, shadow-item-not-a-dict, non-dict node. TryGetLong & ExtractObjectId all branches. Percy.Test/FakeWebDriver.cs: FakeChromeWebDriver gains an optional pluggable CdpResult delegate so closed-shadow tests can return real DOM.getDocument / DOM.resolveNode / Runtime.callFunctionOn payloads; default behavior unchanged. Percy/Percy.cs (one removal, behavior-preserving): EnumerateIframes had an `else if (item is Dictionary<string, object>)` fallback after `if (item is IDictionary<string, object?>)`. Nullable reference annotations are erased at runtime, so IDictionary<string,object?> and IDictionary<string,object> are the SAME runtime type — every Dictionary<string,object> the in-browser enumerate script can return always matches the first arm, making the else-if provably unreachable dead code. Verified empirically (the `is` check is True at runtime). Removed it (replaced with an explanatory comment) so 100% is reached with real tests and zero coverage exclusions/pragmas; the removed path could never execute, so runtime behavior is identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings percy-selenium-dotnet to parity with the canonical Percy CORS iframe + closed shadow DOM feature set.
Implemented
data-percy-ignoreattribute opt-outignoreIframeSelectorsoptionIsUnsupportedIframeSrcPercyContextLostExceptionrecovery mergesPartialCaptureExposeClosedShadowRoots)Skipped
Reference
Mirrored from percy/percy-nightwatch#869 (PER-7292-add-cors-iframe-support); CDP from percy/percy-playwright#609.
Test plan
dotnet test)🤖 Generated with Claude Code via /percy-sdk-sync