Skip to content

fix(e2e): unbreak the harness suite and make its assertions real - #120

Open
CAMOBAP wants to merge 8 commits into
masterfrom
fix/e2e-harness-test-timeout
Open

CAMOBAP wants to merge 8 commits into
masterfrom
fix/e2e-harness-test-timeout

Conversation

@CAMOBAP

@CAMOBAP CAMOBAP commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

The e2e harness suite had been failing on master on every run since at least 2026-08-16.
Fixing it turned up three separate problems, one of which means the suite has never actually
verified anything
.

1. The harness was installed unpinned

__scripts__/setup-e2e-host.js regenerates the host app from scratch on every CI run and
installed react-native-harness with no version constraint, so each run resolved to whatever
was newest at that moment. Upstream added a per-test testTimeout defaulting to 5000ms
(1.0.0 had no per-test timeout at all), and the suite went red with no commit in this repo.

Pinned to 1.6.0. This is the root cause — without it, any timeout fix is temporary.

2. The image assertions could not fail

failureThreshold: 5,
failureThresholdType: 'percent',

failureThresholdType: 'percent' compares differences / totalPixels — a 0–1 ratio — so 5
meant 500%, which no image pair can exceed. Every green e2e run in this repo's history
passed vacuously.

Now 0.01 (1%), and both screenshots are cropped to the widget frame. On a full-screen shot
the widget is ~11% of the pixels and its light card is close to white, so even a missing widget
changed only ~0.6% of the screen. Cropped, against the light baseline a removed widget differs by
~5.5% and the dark theme by ~49%. The four baselines were re-recorded on CI's emulator and
simulator for the new dimensions.

3. Each case slept through the load instead of waiting for it

await render(<WidgetFixture theme="light" />, { timeout: WEBVIEW_LOAD_MS }); // 10s
await new Promise((r) => setTimeout(r, WEBVIEW_LOAD_MS));                    // + 10s

render's timeout only covers React commit plus a double rAF — it does not wait for the
WebView to fetch anything, so the 10s there was decorative and the real wait was the blind
10s sleep. ~94% of each test's duration was that sleep.

Replaced with a signal-driven wait on the token. The test sitekey passes without a challenge, so
the widget's final state - the checked checkbox the baselines show - arrives together with it. The
SDK's internal ready event is not enough: it only means hcaptcha.render() returned, and the
iframe can still be blank on the network after it. The wait also inspects onMessage on every
poll, so an SDK error fails the test with the SDK's own description rather than diffing a
screenshot of a spinner. This PR does not change the SDK itself.

The readiness cap is 20s, deliberately above the SDK's own 15s loadingTimeout: when the
page never comes up we want loading timeout reported through onMessage, not the harness
killing the test with an opaque message.

testTimeout stays as a backstop (120s by default since #122, overridable via HARNESS_TEST_TIMEOUT) — it must live in rn-harness.config.mjs, which is
where the harness actually reads it.

Stability

Five sequential CI rounds on 58135e3 (Android ×5, iOS ×4 — 18 assertions): all green, no
retries, no flakes.

                  min     max     mean
Android  light   2761    5244     4287
Android  dark    2790    4942     3767
iOS      light   4089    6025     4750
iOS      dark    1859    2531     2133

Durations fell from ~10.7s to ~1.9–6.0s, and worst case is ~10% of the 60s backstop. The
spread is the intended consequence of waiting on a signal rather than a clock — runner noise
now shows through in duration, not in outcome.

Most importantly: these are the first runs in which the baselines were genuinely compared.
They held on both platforms.

🤖 Generated with Claude Code

CAMOBAP and others added 5 commits October 5, 2026 00:18
Each case waits up to 10s for the WebView to load and then sleeps another
10s before screenshotting, so it needs more than 20s. No testTimeout was
configured anywhere, so Jest's 5s default applied and the first test was
killed mid-sleep every run - the suite has been red on master since at
least 2026-08-16.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit put testTimeout in jest.harness.config.mjs, which has
no effect. @react-native-harness/jest resolves it as

  session.config.testTimeout ?? projectConfig.testTimeout ?? globalConfig.testTimeout

and session.config is the rn-harness config, whose schema declares
testTimeout with .default(5000). Being a Zod default it is always
populated, so the first branch always wins and the Jest value is never
consulted - CI still timed out at exactly 5000ms.

Move it to rn-harness.config.mjs next to bridgeTimeout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The host app is regenerated from scratch on every CI run, and the install was
unpinned, so each run resolved react-native-harness to whatever was newest at
that moment. That is how the suite went red without a commit here: 1.0.0 had no
per-test timeout at all, and a later release added `testTimeout` with a 5000ms
default, which the 10s sleep in darkTheme.harness.js cannot meet.

Pinned to 1.6.0 - the version CI was already picking up - so upstream default
changes arrive as a deliberate bump instead of a surprise failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`failureThresholdType: 'percent'` compares a 0-1 ratio, not a percentage:

    const percentage = differences / totalPixels;
    pass = percentage <= failureThreshold;

    - @react-native-harness/bridge/src/image-snapshot.ts:142-148

So `failureThreshold: 5` meant 500%, and since the ratio cannot exceed 1, both
assertions passed for every possible pair of equally sized images. The only
failures they could ever report were a dimension mismatch, a null screenshot or
a timeout - the rendering itself was never checked.

Set to 0.01, i.e. 1% of pixels. The widest signal these full-screen shots carry
is the light baseline against the dark one, measured at 5.6% of the screen on
Android and 6.3% on iOS, so 1% leaves room for antialiasing while still catching
a widget rendered in the wrong theme.

This is the first run in which these baselines are actually enforced, so CI is
where we find out whether they still hold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each case slept a flat 10s and then screenshotted, which is where almost all of
the runtime went: on the last green run the two cases took 11074ms and 10652ms,
so roughly 94% of each was the sleep. The sleep was also the only thing standing
in for readiness - `render()` resolves after the React commit and a double rAF,
not after the WebView fetches anything - so the number had to be large enough to
cover a network load nobody had measured.

Waits on a real signal instead. The SDK tears down its loading overlay when the
page posts its first message, so the overlay now carries a testID and the test
polls until it is gone. The budget is 20s, chosen to sit above the SDK's own 15s
loading timeout so a page that never comes up is reported as `loading timeout`
through onMessage rather than as an unexplained harness kill; the collected
messages are checked on every poll so a failure fails the test with the SDK's
own description.

Readiness only means `hcaptcha.render()` returned, so the widget's images may
still be painting. Rather than swap one guessed delay for a smaller one, the
test polls until two consecutive frames are byte-identical, and screenshots
anyway if they never settle.

A healthy run should now cost readiness plus a couple of hundred ms rather than
a flat 10s, and a broken one should say why.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@CAMOBAP CAMOBAP changed the title fix(e2e): give the harness tests a timeout they can actually meet fix(e2e): unbreak the harness suite and make its assertions real Oct 18, 2026
@CAMOBAP
CAMOBAP marked this pull request as ready for review October 18, 2026 09:50
e271828-
e271828- previously approved these changes Oct 24, 2026

@e271828- e271828- left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update: the light snapshot can pass with the widget missing, and readiness can settle before the iframe loads.

name: 'hcaptcha-light',
threshold: 0.15,
failureThreshold: 5,
failureThreshold: FAILURE_THRESHOLD,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The light test still accepts a missing widget: replacing its Android baseline checkbox with the white background changes only 0.385% of pixels, below this 1% threshold. Crop to the widget or lower the threshold.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Confirmed, thanks. Measured on the old baselines: blanking the whole widget frame changed ~0.6% of the screen (0.62% Android, 0.67% iOS), so a missing widget passed.

Fixed in f00a908 by cropping both screenshots to the widget frame, and re-recorded the baselines on CI's emulator/simulator in 70fd0a9. Against the new light baseline, a removed widget now differs by ~5.5% and the dark theme by ~49%, both well above the 1% threshold. The suite passes against them on both platforms.

while (Date.now() < deadline) {
await new Promise((resolve) => setTimeout(resolve, POLL_MS));
const current = (await screen.screenshot())?.data;
if (sameBytes(previous, current)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two identical frames can still be a blank iframe waiting on the network. The ready event only means hcaptcha.render() returned, so this can finish before the checked baseline is rendered. Wait for the test key’s token before settling the screenshot.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. The overlay goes on the ready event, which only means hcaptcha.render() returned, and two identical frames of a still-blank iframe satisfied the settle check.

Fixed in f00a908: the wait now blocks on the token. The test sitekey passes without a challenge, so the checked state in the baselines arrives with it, and the paint-settle check still runs afterwards for the checkmark animation. The overlay testID that only served the old signal is removed, so this PR no longer touches Hcaptcha.js. Still holds after merging v5 (#122): without prewarm the widget still auto-verifies and delivers the token the same way.

@e271828-
e271828- dismissed their stale review October 24, 2026 11:00

Second static pass found false-positive snapshot matching and premature readiness. See my follow-up review; withdrawing the earlier approval.

CAMOBAP and others added 3 commits October 24, 2026 14:05
Addresses the review on #120.

The readiness wait watched the loading overlay, which goes when the SDK's ready
event arrives - and that only means `hcaptcha.render()` returned. The iframe can
still be blank on the network after it, and two identical frames of a blank
iframe satisfied the paint-settle check. The test sitekey passes without a
challenge, so the state the baselines show arrives together with the token; the
wait now blocks on that. The overlay testID added for the old signal is no
longer used and is removed, so this PR no longer touches the SDK.

Full-screen shots also let a missing widget pass: it is ~11% of the screen and
its light card is close to white, so removing it changed ~0.6% of the pixels,
under the 1% threshold. Screenshots are now cropped to the widget frame, where
the same loss is ~5.5%.

The cropped shots have different dimensions, so the four baselines are removed
here and re-recorded by CI in the next commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Recorded by CI run 40122844422 on f00a908, on the same emulator and simulator
the suite compares against. Checked by eye: each shows the checked widget in
its theme. Against the light baseline, a removed widget now differs by ~5.5%
and the dark theme by ~49%, both well above the 1% threshold.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-timeout

# Conflicts:
#	__e2e__/rn-harness.config.mjs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants