Skip to content

Test harness: browser resolution in TestMain, WAND_BROWSER_* selection, no flaky skips #53

Closed

Description

@Fly-Playgroud

Parent

Spec: #33 (Baseline release). Section numbers refer to its Implementation Decisions.

What to build

The suite launches browsers exactly the way users do and CI chooses the browser with environment variables; a red run always means a real bug because no flaky skip, fixed port or public-network dependency remains. Spec section 12; ticket #15, #20.

Acceptance criteria

  • The root suite's init() preload is gone; TestMain runs Browser resolution once and fails with the resolved path in the message; every run prints the browser path and version actually used.
  • Tests launch only through launcher.New() (plus the User mode test); no test-only browser knob exists; CI selects with WAND_BROWSER_*.
  • The pooled tester count follows GOMAXPROCS; the ci-test wrapper remains the entry point setting GODEBUG=tracebackancestors=100 for leakcheck.
  • The two flaky-skipped tests (page navigate error on Windows, hijack mock whole response without body) are root-caused and un-skipped on every Tier 1 OS, or deleted with the covering test named; the dead tests (native drag, old browser, lint ignore, CDP mass browser close) are deleted; the environment guards stay (fonts container-only; binary size non-Windows non-container with the threshold lowered to lock in the leakless removal; profile dir flag-gated).
  • No test uses a fixed port; no test reaches the public internet except the managed-browser download (asserted from the CDP log or a network-off run).
  • A deliberately failing local run leaves no browser or temporary user-data directory behind; the Gate is green.

Blocked by

Activity

  1. Fly-Playgroud commented on Sep 6, 2026

    @Fly-Playgroud
    ContributorAuthor

    Two flakes observed in the Gate on 2026-09-06, both under -race on ubuntu-latest, both green on a rerun of the same SHA, neither named in the acceptance criteria yet. Recording them here so the harness rewrite root-causes them rather than leaving reruns as the fix:

    Both runs carried only leaf-package and README changes (#74, #75), so neither is a regression signal.

  2. Fly-Playgroud commented on Sep 6, 2026

    @Fly-Playgroud
    ContributorAuthor

    Input from #43 (PR #76): the root flake TestScrollScreenshotErrors: leaking MockClient.stub.

    Three Gate hits so far, every one on a push run and green on gh run rerun --failed: main after PR #75 (4232d3c), then both commits of PR #76 (2ddd70a, 6c6c7e7). The pull_request run of the same SHA passed each time. Nothing in those diffs touches the test or ScrollScreenshot.

    Mechanism, read from the code rather than seen in a log: TestScrollScreenshotErrors (page_test.go) wraps six MustScrollScreenshot calls in g.Panic, each after stubbing the first call of one CDP method (stubErr / stub, setup_test.go). A stub is consumed only when the stubbed method is reached. If MustScrollScreenshot panics for another reason first, g.Panic is satisfied all the same (it accepts any panic and prints no value), the stub stays armed, and the next stub or the tester cleanup's setCall(nil) reports leaking MockClient.stub. That is why the job log carries this one line and nothing about the real failure.

    Two candidates for the real failure, neither confirmed: the unstubbed Page.captureScreenshot (FromSurface: false in page.go) failing under -race with four pooled browsers on the 4-vCPU runner, or Page.getLayoutMetrics reading the 4500 px scroll-y.html fixture before layout, since Page.Navigate returns right after Page.navigate responds and does not wait for load.

    Suggested shape of the fix, for this ticket's "a red run always means a real bug": make each g.Panic block assert that the recovered value is the stubbed error, so an unrelated panic fails with its own message; and wait for load before the first stub. TestPageWaitIdle hanging to the TimeoutEach panic under local load (28 pooled browsers on Windows, passes alone) is the other pre-existing hang seen while verifying #43.

  3. Fly-Playgroud commented on Sep 6, 2026

    @Fly-Playgroud
    ContributorAuthor

    Observed while implementing #45 (PR #79), for the system-browser suites this ticket and #61 will run: on Windows 11 with the branded Google Chrome 152 as the System browser, lib/launcher's TestManaged failed two runs out of three at the wait for the crashed browser's user-data directory to be removed (15 s budget; the branded Chrome keeps the directory busy after Browser.crash), while the same test passed three out of three on Chrome for Testing 152.0.7977.82. The Gate now pins WAND_BROWSER_BIN to the Managed browser, so it does not see this; a system-browser run on Windows will.

  4. Fly-Playgroud commented on Sep 6, 2026

    @Fly-Playgroud
    ContributorAuthor

    The Gate's load flakes, collected from the pull requests that hit them, so that this ticket's "a red run always means a real bug" covers them. Each has failed on one sibling run of a SHA while the other passed, and each passes alone; the rerun of the failed job was green every time.

    Test Seen on Mechanism, as far as read from the code
    TestPageWaitIdle (root): [wand_test.TimeoutEach] TestPageWaitIdle timeout after 1m0s, the whole binary panics and the coverprofile is lost PR #78 (three pull_request runs in a row), locally twice on 2026-09-06 MustWaitIdle is WaitIdle(time.Minute), whose JS helper is requestIdleCallback(resolve, {timeout}) with that minute as the fallback; the harness TimeoutEach is also one minute, so when a loaded renderer never goes idle the JS fallback and the test timeout race at 60 s and the test panics first. A shorter WaitIdle timeout than TimeoutEach, or waiting for what the click actually produces, would fix it.
    TestPageErr (root): the harness cleanup at setup_test.go:199 gets No target with given id found from TargetCloseTarget, followed by the sync: negative WaitGroup counter panics of TestSetUserAgent's server that follow any failure PR #78, PR #79 (push run of 49ce337) A target gone between TargetGetTargets and the close; the cleanup should tolerate a target that closed itself.
    TestScrollScreenshotErrors (root): setup_test.go:268: leaking MockClient.stub PR #75 (push run on main after merge), PR #76 (push run of both commits) The test wraps six MustScrollScreenshot calls in g.Panic, each after stubbing one CDP method; a stub leaks when the call panics for another reason before the stubbed method is reached, and g.Panic accepts any panic, so the real cause never prints. Candidates: the unstubbed Page.captureScreenshot (FromSurface: false) failing under -race load, or Page.getLayoutMetrics seeing the 4500 px fixture before layout, since Navigate returns right after Page.navigate. A fix asserts the panic value against the stubbed error and waits for load before stubbing.
    TestCrash (lib/cdp): panic: Fail in goroutine after TestCrash has completed, under -race PR #74 (pull_request run) Not read; a goroutine of the crashed client reports after the test returned.
    TestManaged (lib/launcher): the wait for the crashed browser's user-data directory to be removed exceeds its 15 s budget, then the second handshake gets a net.OpError instead of the expected BadHandshakeError Locally on Windows 11 with the branded Google Chrome 152 as the System browser, two runs out of three; three out of three green on Chrome for Testing (comment above) The branded Chrome keeps the directory busy after Browser.crash. The Gate pins WAND_BROWSER_BIN to the Managed browser since #79, so only a system-browser run sees it.

    None of these has an issue of its own; they are listed here because this ticket owns the harness and the no-flake rule (spec #33, section 12). If the harness work here does not reach them, they need a ticket of their own before #64 (three consecutive green Nightlies).

  5. self-assigned this
    on Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

ready-for-agentFully specified, ready for an AFK agent

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions