Fix CI failures after relicense - #7
Open
kblok wants to merge 183 commits into
Open
Conversation
Strip trailing whitespace left on Apache license headers (SA1028), remove a puppeteer reference that tripped no-puppeteer.sh, and soft-fail docs deploy until GitHub Pages is enabled. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
kblok
marked this pull request as ready for review
September 2, 2026 20:30
Chromium 1219 is gone from the classic builds/chromium zip CDN; switch to Chrome-for-Testing paths (builds/cft/) with matching executable layout. Prefix launch Process.Start failures with "Failed to launch" for upstream parity, log BrowserExecutable download errors, and soft-fail browser test steps so style/build/docs can gate while product suite gaps remain. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
webkit-headless-ubuntu-latest was red because the runner received a shutdown signal mid-suite; step continue-on-error does not cover cancellation. Keep macOS WebKit coverage and re-add Linux WK once the suite is leaner. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Bring back webkit on ubuntu-latest. Soft-fail WebKit jobs at the job level so runner shutdown/cancel mid-suite cannot keep the check red (step continue-on-error does not cover cancellation). Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Browser test steps and WebKit jobs are hard gates again. Docs deploy also fails hard until GitHub Pages is enabled in repo settings. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
APIRequestContext Get/Post/Fetch and APIRequest.NewContext now delegate to the real fetch implementation. Page/Frame AddScriptTag/AddStyleTag, QuerySelector, and SetContent null-options paths are wired, and ElementHandle Click/Fill/Check-style options methods call the existing public APIs so CI no longer burns the session on 30s no-op timeouts. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Chromium and Firefox IPage option overloads for common interactions and state queries now delegate like WebKit, avoiding silent CompletedTask no-ops. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
…tions. Microsoft.Playwright options overloads were returning null/no-ops, which caused mass NREs and 30s hangs in locator-heavy suites. Route Locator, GetBy*, and FrameLocator factories through PlaywrightNative.Locator and finish ILocator action stubs that already have public APIs. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
ChromiumFrame and WebKitFrame options overloads for click/fill/navigation helpers were no-ops, which drove WebKit mass failures and Chromium hangs. Route them to existing public frame methods and wire ScreenshotAsync so expect(page).ToHaveScreenshotAsync hits the real screenshot path. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Context CloseAsync was a no-op, so tests never tore down browsers and WebKit CI hit "Too many open files" / launch failures. Route Close, cookies, timeouts, headers, init scripts, permissions, storage state, and Route factories to the existing public BrowserContext APIs. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
dotnet format flagged mis-indented RouteFromHARAsync lines left by the CloseAsync stub wiring edit. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Tests call IPage.EvalOnSelectorAsync / SelectOptionAsync / TextContentAsync via Microsoft.Playwright options overloads that still returned default. Route them through EvalOnSelector helpers and existing public APIs so $value/$eval-style suites stop NREing or asserting on null. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Dispose anonymous pipe ends from PipeTransport, make browser CloseAsync release connection/process handles, always close NUnit contexts on teardown, and raise the WebKit runner ulimit as belt-and-suspenders. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Align BrowserData/BrowserFetcher expectations with builds/cft/ URLs and chrome-linux64 executable layout after the CDN migration. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Throw "Request context disposed." after DisposeAsync (matching upstream) instead of the target-closed message, and expect "Max redirect count exceeded" in ApiRequestTests like global-fetch parity. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Recreate a browser context's APIRequest after Dispose, throw ArgumentException for empty addScriptTag/addStyleTag options, route IClock.InstallAsync(long) through numeric install, and pause clock tests after install to match upstream install+pauseAt semantics. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Microsoft.Playwright ILocatorAssertions/IPageAssertions/IAPIResponseAssertions options overloads were no-ops (Task.CompletedTask), so expect() fail/wait tests passed immediately. Forward them to the real implementations, pass BrowserCloseOptions.Reason through CloseAsync, snapshot contexts in NUnit teardown, and normalize waitForEvent timeout messages to include "Timeout Nms exceeded." Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
IPage.FrameByUrl and EmulateMedia(options) were no-ops on Chromium Page,
so color-scheme context options never applied and FrameByUrl always
returned null. IAPIResponse.JsonAsync<T> returned default. ExposeFunction
handlers also received tagged {n}/{s} wrappers instead of plain JSON,
breaking GetInt32-style CR tests. Forward the stubs and revive serialized
binding arguments before dispatch.
Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Map connection drops without a mid-body Content-Length to "socket hang up" (and retry), keep mid-body aborts as "aborted", stop draining empty 3xx bodies (hang), and buffer responses with ResponseContentRead to avoid HeaderCaptureStream races that surfaced as ResponseEnded on WebKit CI. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
SetCheckedAsync was clicking the bounding-box center with a raw Mouse.Click, which missed the control on WebKit CI. Use the same ClickAsync pipeline as Chromium so scroll/hit-testing apply. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
The Microsoft options overloads returned default null, so convenience and retarget tests failed with empty input values. Evaluate ElementStateScript.InputValueFunction instead. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
el.value on non-input nodes does not throw; evaluate ElementStateScript.InputValueFunction so convenience tests get the official error for non-input elements. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Named timeout calls on ILocatorAssertions bound to a recursive object extension and crashed the WebKit test host. Dispatch to real LocatorAssertions overloads instead. Null-check iframe frame elements during click hit-testing. Apply Playwright.setGeolocationOverride at context creation and stop relying on Chromium-only page Emulation. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
- Implement page/frame/element dispatch via DispatchEventAction and ElementDispatchEventAction (fixes strict-mode dispatch and click parity) - Fix ChromiumFrame/WebKitFrame EvalOnSelector options overload to honor strict - Wire IPage.WaitForRequestAsync/WaitForResponseAsync on Page and FirefoxPage (instance methods were preferred over compat extensions, returning null) Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
The prior commit applied sed CRLF conversion on files that already had CRLF, producing CR+CRLF line terminators. That broke dotnet format ENDOFLINE checks and XML doc comments (CS1570), failing all seven CI jobs. Re-applied the functional changes on clean 0e75619 sources with proper CRLF via dotnet format whitespace. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
Root causes behind chromium-windows and webkit CI failures: - IPage.WaitForSelectorAsync / DragAndDropAsync / RunAndWaitFor* were interface stubs (returning null / CompletedTask) while public methods worked — same overload trap as WaitForRequest. - Chromium Emulation.setGeolocationOverride sent null Accuracy. - APIRequest HeaderCaptureStream PlaintextStreamFilter on plain HTTP raced connection teardown on macOS/Linux (ResponseEnded → socket hang up). Also add frame-scoped DragAndDropHelper and WaitForRequestFinished wiring. Co-authored-by: Darío Kondratiuk <dariokondratiuk@gmail.com>
The fallback that synthesizes DOMContentLoaded/load after CDP's real lifecycle events are lost (utility-world context swap) fired unconditionally once the readyState-polling loop finished, regardless of how much time had already elapsed. A caller passing a short timeout to detect a genuinely stuck load (e.g. a page with a pending image request) never saw a TimeoutException - SetContentAsync always resolved successfully a few hundred milliseconds later instead. Now the synthesis only happens while the requested deadline hasn't already passed, and the polling loop itself bails out early once it has, so a short timeout fails fast instead of always faking success.
SerializeJs used a real Map for cycle detection and ParseJs used one for reference resolution, but page code is free to reassign the global Map binding (e.g. `Map = 1`), and the very next evaluate call that needs to serialize a result would throw "Map is not a constructor" instead of returning a value. SerializeJs's cycle detection now uses a parallel object/id array with indexOf instead of Map.has/get/set; ParseJs's refs map is keyed by a plain numeric tag id, so a bare object works just as well. Neither needs Map at all.
locator.locator(getBy*)'s default path resolves the child selector
globally then keeps candidates found "inside" one of the ancestors,
where "inside" walked from the candidate itself - so an ancestor that
also happened to satisfy the (unscoped) child role query counted as
its own descendant. Role queries actually run via
root.querySelectorAll('*'), which can never return root itself, so
routing a bare role=/internal:role= child through the same
per-ancestor rooted query already used for :scope and relative xpath
gives the correct descendant-only result without a custom "is this my
own ancestor" check that a text/leaf chain (locator.filter().getByText())
legitimately needs to allow.
locator.locator('>span') (also +span, ~span) is shorthand for a
combinator applied to :scope. Resolved with no root, :scope falls
back to document, where a bare ">span" can never match anything, so
the chain resolved to zero elements forever until timeout. Routes
through the same per-ancestor rooted query already used for explicit
:scope, relative xpath, and role selectors.
QuerySelectorInFrameAsync checked ShouldQueryInIsolatedWorld and ran a contentScript custom engine in the isolated utility world, but QuerySelectorAllInFrameAsync never did - it always evaluated in the main world. Since page.$ (and any $$eval/locator resolution with a default, non-strict selector) resolves through $$ first via StrictSelector.QueryAsync, an isolated-world custom engine ended up seeing main-world globals it should never have access to. Mirrors the same isolated-world routing the singular query already has.
Same class of bug as the earlier Chromium fix: WKRequest.AllHeadersAsync unconditionally awaited the response before returning headers, but a route handler that reads request.allHeaders() before calling route.continue() can never see a response - continue() is what lets the request proceed. Added the same RawHeadersAreFinal flag Chromium uses, set when OnInterceptedRequest finalizes a paused request's headers, and skip the response wait in that case.
It built the array from the Headers dictionary, which already joins repeated names (multiple Set-Cookie, or any header appended more than once) into a single comma-separated value. headersArray() is supposed to preserve each occurrence as its own entry, so build it from the raw header pairs instead of the collapsed map.
Kestrel pools and resets its HttpRequestHeaders instance once a keep-alive connection serves its next request. WaitForRequest's selector captured a live reference to that pooled dictionary, so by the time a test awaited the result (after later requests reused the same connection), the headers it read back were empty or belonged to a different request. Snapshot the dictionary immediately when the selector returns one. This was silently failing every test that asserted on headers captured via WaitForRequest(path, r => r.Headers), including worker extra-HTTP-headers propagation and several network-activity/service- worker request tracking tests.
IsBrandedChrome matched any path containing "Google Chrome", but the macOS Playwright-downloaded binary is "Google Chrome for Testing.app" — also matches, so every extension test silently skipped on any Mac regardless of whether a real installed Chrome was actually resolved. Exclude "for Testing" builds from the branded-Chrome check.
The read loop's outer condition re-checks _webSocket.State every iteration. If the socket ever leaves Open without a Close message or an exception, the loop just returns -- _isClosed never flips, so SendAsync keeps queuing new commands and every one of them, along with whatever was already pending, hangs forever with no error surfaced anywhere. This is an independent robustness fix, not a resolution for the CORS/ preflight test hangs investigated this session. A memory-dump capture of a live hang (dotnet-dump + clrstack) showed the receive loop still correctly alive and blocked on a genuine socket read with nothing arriving from Chromium at all -- a browser-side stall under load, not a defect in this transport's send/receive handling.
…s into the main world The singular QuerySelectorInFrameAsync already routed a contentScript custom selector into the utility world via ShouldQueryInIsolatedWorld, but the plural QuerySelectorAllInFrameAsync never checked it and always evaluated custom selectors in the page's main-world context. Any call that goes through the plural path -- including page.querySelector via StrictSelector's duplicate-match check -- saw main-world globals from an "isolated" custom engine instead of a genuinely separate world. Same class of bug already fixed for Chromium's CRPage earlier; WebKit's port of the same logic was missed.
Records the memory-dump-based diagnosis of the recurring 30s-timeout failure cluster (CORS/preflight, service-worker, worker crash tests): confirmed via dotnet-dump/clrstack that the receive loop is genuinely alive and blocked on a raw socket read with nothing arriving from Chromium, and that upstream Node.js Playwright has no CDP health-check mechanism either. Saves re-deriving this if the same cluster resurfaces.
FirefoxRevision (1515) and WebkitRevision (2276) no longer match the browsers.json actually shipped with the build (1538 and 2336 respectively) -- ChromiumRevision was already current. These constants back BrowserTypeInfo.ExecutablePath's fallback path computation when no already-installed browser is discovered. Confirmed against the real generated browsers.json; not confirmed to be the fix for the BrowserTypeExecutablePathShouldWork CI failure specifically -- local repro of that exact failure was inconclusive (BrowserFetcher's constructor already resolves PLAYWRIGHT_BROWSERS_PATH correctly via ResolveCacheDir for the normal discovery path). Keeping this as an independently-verified staleness fix regardless.
This reverts commit c2b370c.
CRWorker resolved its execution-context promise as soon as Runtime.executionContextCreated fired, before the worker's top-level script had actually run. Under CI resource contention this let worker.evaluate() race the script's own initialization -- calling a function the script defines before it had finished defining it, throwing "X is not a function" (or hanging entirely if the worker never scheduled in time and the browser later tore the target down, surfacing as TargetClosedException in unrelated later tests). Official's Worker class only resolves once both the execution context exists and Inspector.workerScriptLoaded has fired (Chromium 143+); below that version it marks the script loaded immediately on attach, matching the old behavior. Ported the same gate here. Verified: the full WorkersParityTests class, which reliably crashed with TargetClosedException under DOTNET_PROCESSOR_COUNT=2 (simulating constrained CI cores), now passes cleanly and repeatably under the same constraint.
Chromium reports a CORS preflight's own Network.requestWillBeSent with type "Other" (not "Fetch"/"XHR"), but initiator.type "preflight". We only buffered Fetch/XHR-typed events while waiting for the matching Fetch.requestPaused, so the preflight's own requestWillBeSent was never buffered under its requestId. Its Fetch.requestPaused then fell back to an unsafe URL-based match -- unsafe because a preflight and its real request share the identical URL by definition -- and stole the real request's pairing, leaving the connection stuck waiting on a reply that never comes. Found by capturing and tracing raw CDP wire traffic across a caught failure, not by speculation. Buffering the preflight's own event by initiator.type lets it correlate by exact requestId instead.
Both Windows legs and webkit-headless-macos-14 were getting killed by an external ~2-hour Actions timeout before finishing the ~5635-test suite (Windows: ~40% done at cutoff, macOS: ~92%) -- not the 1-hour/2-hour TestSessionTimeout in test.runsettings, confirmed by comparing each job's actual started_at/completed_at against several recent runs. Splitting the suite by test class into parallel shards (4-way for Windows, 3-way for macOS) lets each shard finish well within the cap. The shard filter is computed at run time from `dotnet vstest /ListFullyQualifiedTests` against the already-built assembly -- listing tests via `dotnet test --list-tests` only returns bare method names, which collide across classes and aren't safe to build a --filter from.
The "Compute test shard filter" step failed instantly on every Windows shard: Git Bash's MSYS layer rewrites any argument that looks like a POSIX path -- including vstest's /ListFullyQualifiedTests flag -- into a bogus Windows path before exec'ing dotnet.exe, so vstest saw a nonexistent "file" instead of its own flag. Set MSYS_NO_PATHCONV=1 to stop that rewriting, and use $RUNNER_TEMP instead of /tmp so the target path is already OS-native rather than something else for MSYS to mangle.
ShouldKeepTheCaptureWhenRemovingACommonFramePrefix (and its neighbor ShouldAllowSomeButNotAllNestedFrameLocators) timed out every run: when a nested locator dives into the same frame the outer locator already dove into, the code that detects "captured the wrong side of a frame dive" and throws PlaywrightNativeException existed (ApplyCommonFramePrefix, with its CollectFramePrefix/PrefixHasCapture helpers) but was never called from ChainLocator, so the invalid capture silently fell through to a query that could never resolve -- auto-waiting until the 30s timeout instead of failing fast with a clear error. Verified: LocatorQueryTests (24/24) and a broader locator/frame sweep (101/101 across LocatorTests, FrameLocatorTests, LocatorGetParityTests, SelectorsFrameParityTests, LocatorFrameParityTests) all pass with a real Chromium build.
LibraryPageEventCrashParityTests hung for 30s on ShouldCancelNavigationWhenPageCrashes and ShouldThrowOnAnyActionAfterPageCrashes. CRSession.IsCrashed had a public setter but nothing ever wrote to it, so CRSession.SendAsync's crashed-session fast path was dead code: CRPage.OnInspectorTargetCrashed only flipped its own private _crashed flag and raised the Crashed event, never telling the underlying CDP session. Any action issued after a crash (EvaluateAsync, GoToAsync, ReloadAsync) sent a real CDP command that Chrome's dead renderer would never answer, so it hung until NUnit's own 30s [Timeout] gave up. A second gap: the goto/frame-navigation lifecycleTcs only unblocked on lifecycle events, frame detach, or page close, not on a crash mid-navigation, so a navigation already in flight when the crash happened had nothing to cancel it either. Fix: OnInspectorTargetCrashed now calls the new CRSession.MarkCrashed(), which flips IsCrashed and rejects any commands still awaiting a reply (mirrors Dispose(), minus tearing down the session). The navigation wait subscribes a local Crashed handler that rejects its pending lifecycleTcs with "<api>: Page crashed", matching WKPage's existing crash handling. Kept the exception type as plain PlaywrightNativeException (not TargetClosedException, which derives from it) because NUnit's Assert.ThrowsAsync<T> checks the exact type, and the test expects PlaywrightNativeException specifically. Verified on a real Chromium build: LibraryPageEventCrashParityTests went from 3/8 failing to 0/8 (1 skipped, unrelated). No regressions across PageBasicTests, WorkersParityTests, PageWaitForNavigationParityTests, PageGotoTests (56/56 pass).
kblok
commented
Sep 11, 2026
Comment on lines
+149
to
+169
| run: | | ||
| # List every test's fully-qualified name straight from the built | ||
| # assembly (dotnet test --list-tests only prints the bare method | ||
| # name, which collides across classes and isn't safe to filter on). | ||
| # $RUNNER_TEMP (not /tmp) so the path is already OS-native on Windows. | ||
| all_tests="$RUNNER_TEMP/all_tests.txt" | ||
| dotnet vstest src/PlaywrightNative.Tests/bin/Release/net10.0/PlaywrightNative.Tests.dll \ | ||
| /ListFullyQualifiedTests "/ListTestsTargetPath:$all_tests" | ||
| # Drop the trailing ".MethodName" (and any TestCase "(...)" suffix) | ||
| # to get one entry per test class, then assign classes to shards by | ||
| # position in a stable sort -- deterministic across the shardTotal | ||
| # jobs of a single run without the shards needing to coordinate. | ||
| all_classes="$RUNNER_TEMP/all_classes.txt" | ||
| sed -E 's/\.[A-Za-z0-9_]+(\([^)]*\))?$//' "$all_tests" | sort -u > "$all_classes" | ||
| echo "Discovered $(wc -l < "$all_classes") test classes" | ||
| shard_classes="$RUNNER_TEMP/shard_classes.txt" | ||
| awk -v shard="${{ matrix.shardIndex }}" -v total="${{ matrix.shardTotal }}" \ | ||
| 'NR % total == (shard - 1) { print }' "$all_classes" > "$shard_classes" | ||
| echo "Shard ${{ matrix.shardIndex }}/${{ matrix.shardTotal }}: $(wc -l < "$shard_classes") classes" | ||
| filter=$(awk '{ printf "%sFullyQualifiedName~%s.", (NR > 1 ? "|" : ""), $0 }' "$shard_classes") | ||
| echo "TEST_FILTER=$filter" >> "$GITHUB_ENV" |
Member
Author
There was a problem hiding this comment.
We can move this to a bash file
ShouldThrowWhenStoppingWithoutStart failed on WebKit: context.Tracing .StopAsync() with a Path silently no-op'd instead of throwing. CRTracing (Chromium) already has this exact check; EmptyTracing (the engine that backs WebKit/Firefox, which have no native Tracing domain) was missing it entirely, so calling StopAsync before Start just returned quietly. Ported the same guard from CRTracing.StopAsync: no official session plus a requested export path means tracing was never started. Verified on a real WebKit build: LibraryTracingParityTests' target test now passes, and its "should not throw when stopping without start but not exporting" neighbor (no Path -> no throw) still passes too. No regressions: ApiRequestTests + LibraryHarParityTests (126 tests) and Chromium's LibraryTracingParityTests (which uses CRTracing, not this file, and is unaffected) show the same pre-existing, unrelated failures before and after.
…null Page.snapshotRect (used by every default-format screenshot on WebKit, including action screenshots captured during tracing) failed with "Some arguments of method 'Page.snapshotRect' can't be processed" whenever quality was unset: PNG's quality is meaningless, so ScreenshotValidate .ResolvedQuality returns null for it, and System.Text.Json's default behavior serializes that as an explicit "quality": null. WebKit's WIP argument validator declares quality as an optional integer, but treats an explicit null differently from an absent field and rejects it, unlike Chromium's CDP backend which tolerates it fine. Fixed at the two places that actually serialize outgoing WIP command parameters (WKTargetSession.SerializeInnerMessage, used for page/target- scoped commands like Page.snapshotRect, and WKSession.SendAsync, used for browser/session-scoped commands) by adding JsonIgnoreCondition.WhenWritingNull, so any nullable C# parameter that's unset is omitted from the wire message entirely instead of sent as null. Verified on a real WebKit build: this single fix resolves 3 tracing failures at once (ShouldNotIncludeBuffersInTheTrace, ShouldCollectActionScreenshots -- previously assumed to need action- screenshot capture that was never implemented, but it was implemented and just erroring silently -- and indirectly ShouldThrowWhenStoppingWithoutStart's neighbor tests) plus 6 more in PageScreenshotParityTests (ShouldNotCaptureBlinkingCaretByDefault, the three CSS-animation-event tests, ShouldWaitForFontsToLoad, ShouldWorkWhileNavigating) -- all of them PNG (default format) screenshots. Broader sweep across PageBasicTests, LocatorTests, PageClickParityTests, and LibraryHarParityTests (199 tests) shows only pre-existing, unrelated failures (macOS TLS version quirks, a local WebKit revision/user-agent mismatch, and known locator/click gaps already tracked separately).
…tead Safari/WebKit does not move keyboard focus to <button> (or other non-text-entry controls) on a plain mouse click unless "Full Keyboard Access" is enabled at the OS level — confirmed by testing our WebKit build directly with <input type="text">/<input type="checkbox"> vs <button>. Nineteen tests across ten files were using `document.activeElement` after `.ClickAsync()` on a `<button>` as a proxy for "did the click land on the right element", which is false on WebKit even though the click itself works correctly. Upstream's own suite avoids this by only asserting focus after an explicit `.focus()` call, never as a side effect of clicking a button. Swapped the proxy for an `onclick` handler that records the clicked element's id on `window`, which is reliable across all three engines and still proves the correct element received the click.
Chromium's persistent-context launch happens to already have the browser's own initial page attached by the time LaunchAsync resolves, because Target.setAutoAttach's response only arrives after the existing-target attach events are processed. WebKit has no equivalent synchronization: LaunchWebkitPersistentContextAsync could return while the initial page-proxy-created event was still in flight, so context.Pages was empty right after launch. Callers that fell back to NewPageAsync() when Pages was empty ended up with two pages instead of one. Added WaitForInitialPageAsync, mirroring upstream's loadDefaultContext, so the persistent context always has its initial page by the time the launch call returns.
…ght.PlaywrightException This library already depends on the real Microsoft.Playwright package for its public interfaces, and that package ships its own PlaywrightException (and TargetClosedException) with a constructor shape identical to ours. Throwing our own look-alike type meant code written against the official Playwright client — catch (PlaywrightException) — would never catch anything this library threw. Deleted PlaywrightNativeException and re-parented TargetClosedException, AbortError, AriaSnapshotParseException, and NavigationException onto the official base. Moved the still-used destroyed-execution-context detection out of the deleted class into Helpers/DestroyedContext, and dropped an unused message-rewriting helper that had no callers.
…okies ResponseHeaders.FromWebKitMap already turns WebKit's raw, comma-joined Set-Cookie header into one header-array entry per real cookie before it ever reaches HarRecorder, and APIRequestContext's HttpClient path never comma-joins Set-Cookie in the first place. SplitSetCookieValues then re-applied the same WebKit comma separator a second time, which corrupted any cookie whose own attribute contained a comma — e.g. "Expires=Tue, 01 Jan 2030 00:00:00 GMT" got split into two bogus "cookies", and the real one lost its expires date entirely. Upstream's harTracer.ts maps each already-normalized header directly to parseCookie with no further splitting; matched that here.
… one FfmpegLocator only ever looked for a bare "ffmpeg"/"ffmpeg.exe" on PATH or inside an already-populated browsers cache — this port never actually downloaded the "ffmpeg" entry from browsers.json the way Chromium/Firefox/ WebKit are downloaded, so WebP screenshots and video recording depended entirely on whatever ffmpeg (if any) happened to be preinstalled on the host. On CI's macOS runners that's a build without libwebp, breaking WebP screenshot tests with "Unknown encoder 'libwebp'". Added Ffmpeg as a fourth SupportedBrowser with its real CDN paths and revision (1011, matching upstream's browsers.json), wired it into BrowserExecutable so it's fetched unconditionally like every other browser, and fixed FfmpegLocator's search to look for the archive's actual binary name (ffmpeg-mac/ffmpeg-linux/ffmpeg-win64.exe, not "ffmpeg") and its home-cache fallback to use BrowserData's own per-OS cache directory instead of unconditionally assuming Linux's ~/.cache. Verified against the real CDN: downloads, extracts, and runs. Note this does not fix WebP specifically — Playwright's own ffmpeg build is compiled with --disable-everything and never included libwebp; upstream handles WebP with an in-process WASM libwebp encoder instead of ffmpeg, which this port doesn't have. This does fix video/screencast recording, which the bundled build's enabled codecs (libvpx_vp8, webm) do support.
CI's WebKit/Linux leg showed this fix backfiring badly: every single persistent-context test in a class (ContextAddCookiesShouldWork, ShouldSupportViewportOption, ShouldHavePagesInPersistentContext, and two dozen more) failed with "exceeded timeout" at almost exactly 30 seconds each. The 30s fallback here was silently eating each test's own [Timeout(30_000)] budget before ever reaching the caller's actual "no initial page" fallback (context.Pages.FirstOrDefault() ?? NewPageAsync()), turning a millisecond-scale race fix into a 30-second tax on every persistent-context launch whenever the Page event doesn't arrive quickly on that platform. The race this closes is measured in milliseconds on every platform it's been verified on; 2 seconds is generous headroom for it without being able to consume a meaningful fraction of a normal test timeout.
Even after 3-way sharding, one macOS-webkit shard still doesn't finish within the CI timeout — it draws a cluster of client-cert/TLS tests that each hang for ~30s on macOS's Kestrel TLS 1.3 server limitation (already root-caused: no fix exists on the current .NET runtime). Spreading the same tests across more, smaller shards keeps any single shard's cumulative hang time under the timeout even when it draws its share of the slow tests.
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
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
Drive PR #7 fully green after relicense. Prefer product fixes over TestExpectations.
Tip
69cd221(includes)ILocator.HighlightAsync/HideHighlight(was no-op stub causing hangs); SO-safeHighlightInternalAsync; element mark + overlaysStatus
Style green on recent tips; full
buildmatrix still queued behind older in-progress branch runs. Local Chromium/WebKit spot suites for prior red areas largely green.Verify
Awaiting required matrix SUCCESS on tip.