Repository navigation
feat(e2e): Maestro E2E harness, RN version CI matrix and iOS build on RN 0.87 - #5096
Merged
Merged
Conversation
Adds test-app/, a react-native-test-app host that renders player events as text markers with stable testIDs, so Maestro can assert on player behaviour without parsing numbers or inspecting video pixels. Scenarios are opened by deep link (rnvtest://scenario/<name>), never by UI navigation. Includes a two-line patch to react-native-test-app 5.4.9: in singleApp mode its MainActivity redirects to a separate ComponentActivity without copying the launch Intent's action and data, which makes Linking.getInitialURL() return null for every Android deep link. Rationale in test-app/patches/README.md. Registers test-app as a workspace and extends postinstall to apply its patches.
Local fixtures so playback tests never depend on the network: an 8 s mp4 (testsrc video + 440 Hz sine), the same clip repackaged as a 2 s-segment HLS VOD, and an intentionally broken manifest for the parse-error path. generate.sh regenerates them with ffmpeg; the outputs are committed (~460 KB) so CI does not need ffmpeg installed.
Ten Maestro flows: mp4 happy path, HLS load and progress, seek on both source types, mute/volume, rate, loop, replay after end, a 404 source and a broken manifest. CONTEXT.md records the decisions behind the harness and the non-obvious behaviours found while building it: iOS deep links needing an AppDelegate hook, useSyncExternalStore bailing out on mutated snapshots, onError not firing for async load failures (listen to onStatusChange too), and the Maestro tap deferral during decoding that dictates how every interactive flow is written. README.md covers running the suite locally and adding a flow.
Two jobs, one per platform: an Android emulator on a KVM-accelerated ubuntu runner and an iOS simulator on macOS. Both serve the local fixtures, run the whole flow directory and upload the JUnit report plus Maestro's recordings. Neither check is marked required yet. CONTEXT.md asks for a run of consecutive clean results first, and Android stability still needs confirming on a clean machine. Known rough edges, addressed in the follow-up that introduces the RN version matrix: the Pods cache step sits after pod install so it never restores, the xcodebuild pipeline ends in `|| true` and swallows build failures, and the fixture server is fetched with npx on every run.
docs runs React 19 but declared no @types/react, so it typechecked against the @types/react 18 hoisted from example/ and test-app/ (both on React 18 via React Native 0.77). Every Docusaurus component failed as a JSX element with "Type 'bigint' is not assignable to type 'ReactNode'", which broke `bun typecheck` for the whole workspace and with it the pre-commit hook. Adds the missing @types/react and @types/react-dom, and pins react/react-dom type resolution in docs/tsconfig.json: with React 18 hoisted at the root, docs and @docusaurus/types each get their own nested copy of @types/react 19, and two physical copies of one version are two distinct types to TypeScript. The @site/* alias is re-declared because "paths" replaces the inherited map rather than merging with @docusaurus/tsconfig. Types only - no runtime dependency changes, so the docs build is unaffected.
A tapOn issued while the clip is decoding is not delivered until the player goes idle, which for the 8 s fixtures means near the natural end. The seek, HLS seek and mute/volume flows waited 5 s after the tap, so they would fail against a correct player. Align them with smoke-rate's 15 s.
…pport Replaces `npx serve`, a network fetch before every job and a flake source. AVPlayer and ExoPlayer both issue Range requests, so the server answers them, including suffix ranges and 416 for unsatisfiable ones. e2e/ and scripts/ are not workspace packages, so lint and typecheck never reach them; a root `test` script and a lefthook hook run their unit tests.
The floor version (0.77) is the repo's default state. Every other version gets an overlay of test-app dependencies plus a committed lockfile under e2e/rn-matrix/<version>/, so CI can install with --frozen-lockfile and never resolve a dependency freshly. The swap is scoped to test-app/; bun nests its react-native per workspace, so the root workspace is untouched. --refresh re-resolves a version's lockfile and always restores test-app/package.json and the root bun.lock afterwards, even on failure.
- junit-summary renders each leg's job summary and tells a truncated report apart from an empty one. - record-result appends one JSON line per matrix row per nightly run and prints the consecutive-green streak that gates marking a check required. A missing, truncated or zero-case report counts as a failure, never as green; an empty quarantine row is the one exception. - nightly-issue keeps exactly one issue per failing row, labelled e2e-nightly: opens it, comments on repeat failures, closes it on green. Failed mutations exit non-zero instead of reporting success. CLI entry points are exported and tested in-process: bun test cannot spawn subprocesses (EBADF from spawnSync under bun 1.4).
One leg per call: select an RN version, build the test app, boot a device, serve fixtures and run the Maestro flows, then upload the report under a name that includes RN version and device. - setup-bun gains a `frozen` input; e2e-setup composes the shared steps. - Android reinstalls the APK with -r, so a cached AVD never runs a stale build, and keys its AVD cache on RN version and API level. - iOS resolves the simulator by device and runtime, fails when the requested runtime is missing, and asserts the booted runtime after boot, so an iOS 18 row cannot silently run on iOS 26. The Pods cache is keyed on the runner image, not runner.os, which is "macOS" on every image. - `blocking: false` sets continue-on-error, keeping the run green while the individual check still reports its real result.
Android runs all three RN versions (0.77, 0.82, 0.87) on API 36; iOS runs once, on RN 0.87 and iOS 26, so a PR spends exactly one of the free plan's five concurrent macOS jobs and five PRs in flight still fit. Branch protection must pin these exact status contexts: android (0.77) / maestro android (0.82) / maestro android (0.87) / maestro ios / maestro The RN 0.87 legs are non-blocking until the library-side fixes for 0.87 land. They render red on every PR in the meantime and must not be added to required checks while they do.
Nightly runs 17 legs: 9 Android (3 RN x API 36/35/34), 6 iOS (3 RN x iOS 26/18) and 2 quarantine legs that run only flows tagged `flaky` and can never fail the run. A matrix-plan job derives both the matrix and the expected row labels from one list, and the report job iterates that list, so a leg that died before uploading anything is recorded as a failing row rather than vanishing. It appends each row to the e2e-results branch and keeps one issue per failing row. The weekly refresh re-resolves e2e/rn-matrix/*/bun.lock and opens a PR. It refuses to force-push over any commit on its branch that it did not both author and commit, and the PR body warns that the e2e gate did not run against it.
e2e/CI_MATRIX_DESIGN.md records decisions D8-D13, following D1-D7 in CONTEXT.md, and lists what must be set up by hand before the first run: the orphan e2e-results branch, the e2e-nightly label, and allowing GitHub Actions to create pull requests. CONTEXT.md now records that the error-404 flow's warm-up works around a cold-launch hang nobody has root-caused, and the README's local-run steps match what CI actually does.
A JSON rewrite in beb3a05 serialized the release-it section titles as \uXXXX escapes. Same content, readable again.
…plan
- react-native-test-app embeds test-app/dist/ and falls back to Metro when it is
missing; no Metro runs in CI, so every leg now runs bun run build:<platform>
(before pod install on iOS).
- matrix-plan counted jq 'length' on {include: [...]} objects and single row
objects, so it always failed its own shape guard; count the row arrays instead.
- Quarantine legs are only scheduled when a flow is actually tagged flaky:
Maestro refuses a tag filter that matches nothing.
- setup-bun keeps frozen (E2E) and shared (docs) node_modules caches in separate
namespaces so a docs job can no longer re-save an RN 0.87 tree under the floor
key; a frozen install failure now says how to regenerate the variants.
- The PR gate no longer carries RN 0.87 rows that are red by design until three
library fixes land; iOS gates on the floor for now (D13 revised), and docs-only
PRs skip the gate.
- Build the APK for x86_64 only, the emulator ABI.
- Comments trimmed to describe current behaviour rather than review history.
…the Linking listener useVideoPlayer runs the setup callback synchronously during the first render when initializeOnCreation is false, so an eventLog.reset() in useEffect ran after the first events (a 404 fails ~30 ms after mount) and wiped them. Reset at the top of setup instead, and key ScenarioScreen on the scenario so a switch remounts cleanly. The e2e-host-ready marker rendered on first paint, before the url listener was attached, so it did not prove what the flows use it for. Render it from a ready flag set after Linking.addEventListener.
…t fix into the Podfile - Remove jest, react-test-renderer, their types and prettier: there are no jest tests and they pulled the whole @jest tree into the root lockfile. - Add a typecheck script so the root typecheck covers the test app. - react-native.config.js only configures android and ios. - react-native-test-app forwards :post_install from use_test_app!, so the fmt C++17 workaround lives in the Podfile (as in example/ios/Podfile) instead of a script that had to be remembered after every pod install. - gradle-wrapper.properties: react-native-test-app rewrites it to the Gradle version it requires on every CLI invocation; commit that state so it stops showing up as a local diff.
import.meta.main only exists on Node >= 22.18; on older Node every script exited 0 without doing anything. Guard on the module path instead. use-rn-version.mjs now prints how to restore the floor, and a pre-commit hook refuses to commit the root bun.lock or test-app/package.json while switched to a non-floor variant.
Keep the decisions and the technical gotchas; drop session logs, run metrics, references to plan documents that are not in the repo, and the description of fixes to workflows that never existed on master. Document the JS bundle step, the revised PR gate, the conditional quarantine legs and the lockfile regeneration requirement.
Regenerated after dropping the unused test-app devDependencies.
…rsions bun nests a variant's react-native under test-app/node_modules, and a later frozen install for another variant (or the floor) leaves that copy in place, so test-app kept resolving the previous version after a switch or a --refresh. Remove it on every switch so the next install starts clean, and say which install to run.
The rows, artifact names, shape guard and flaky detection lived in a bash step inside the workflow, where the only way to exercise them was a real nightly run; that is how the jq 'length' bug got through. scripts/e2e/matrix-plan.mjs now builds the plan and the workflow only calls it. Tests cover the 15/17-row shapes, unique artifact names, the slug derivation the reusable legs use, and flaky-tag detection.
react-native-test-app re-applies the Android config plugin on every Gradle build against the same manifest, so a non-idempotent mod accumulates intent-filters. The plugin's transforms are now exported as pure functions and tested by applying them repeatedly. parseScenario moves to its own module so the deep-link regex every flow depends on has tests. bun test now also runs test-app/, from the root test script and the pre-commit hook.
Seven E2E workflow files only ever execute in CI. actionlint catches expression typos, unknown workflow_call inputs and matrix keys, and runs shellcheck on run: blocks, before a 40-minute matrix does. Runs on PRs touching .github/, and locally from the pre-commit hook when actionlint is installed.
The rules behind the derived markers (paused only after playing, seek-back only after a forward seek landed, loop verified on the third onEnd, the rate cycle, low volume only while unmuted) lived inside ScenarioScreen's player setup callback, entangled with react-native. eventLog.handle() now takes a normalized PlayerEvent and owns that logic; the screen only maps listener payloads onto it. The derived bookkeeping (end count, forward-seek flag) moves into the store and is cleared by reset() with everything else. Behaviour is intended to be identical; the log lines for playback state and volume changes are now formatted from the normalized fields rather than the raw payload. Re-run the flows on a simulator before merging.
First unit tests for the library's JS layer, under bun test like the rest of the repo. VideoError: code/message extraction, view/* routing to VideoComponentError, stack rewriting and passthrough of anything that is not an encoded native error.
Renders the hook with react-test-renderer, which joins the library's devDependencies.
Regenerated after adding react-test-renderer to the library's devDependencies.
…ct 19 No workflow ran bun lint / typecheck / test before; they only ran from the local pre-commit hook. The checks job runs them on React 18, the workspace default. The react-19 job forces React 19.2.8, react-test-renderer 19.2.8 and @types/react 19.2.18 through root overrides in a non-frozen install that is never committed, verifies the library resolves 19.x, and re-runs its unit tests and typecheck. RN 0.82+ requires React 19 and nothing else in the repo exercised it outside the nightly E2E matrix. Simulated locally: 56 tests pass, typecheck clean. It uses oven-sh/setup-bun directly rather than the setup-bun composite so its React 19 node_modules never lands in the frozen cache.
@react-native/typescript-config sets types: [react-native, jest]; the test app no longer depends on @types/jest (its unit tests run under bun test), so a clean install failed typecheck with TS2688. Passed locally only because a stale node_modules/@types/jest was still around.
With the iOS build fixed, 0.87 joins the PR gate: an android (0.87) job alongside 0.77 and 0.82, and the single iOS job moves from the floor to 0.87, where version-specific breakage lands first (D13). The floor's iOS build stays covered by nightly on iOS 26 and 18. The gate still spends one macOS job per PR. Branch protection, once checks become required, needs the new "android (0.87) / maestro" context. The design doc now describes 0.87 as supported and lists iOS 0.87 on a hosted runner and iOS 0.82 as the open risks.
CONTRIBUTING.md still described Jest, Detox and testing only in the example app, and nothing pointed contributors at the unit tests or the Maestro suite. - CONTRIBUTING.md: a "Testing your change" section (unit test vs. Maestro flow, a reproducing flow for E2E-coverable bug fixes, what to do when no test fits, no retries), what CI runs on a pull request, the RN matrix lockfile regeneration after any package.json change, the pre-commit hooks, and the current monorepo layout, commit types and scripts. - Pull request template: a tests reminder, a structured test plan prompt and checklist items for tests, a reproducing flow and local checks.
github-project-automation
Bot
moved this to To Triage
in React Native Video - Public Roadmap
This was referenced Sep 14, 2026
- react-native.config.js: require react-native-test-app directly. The try/catch came from the template, where the package may not be installed yet; here it only hid configureProjects errors behind a broken autolinking config. - rnv-e2e-plugin: drop the picture-in-picture and background-audio options, which no flow exercises, and fail with a clear error when react-native-test-app's manifest has no LAUNCHER activity instead of a TypeError at Gradle settings time. - Tests: assert the full intent filter and that the LAUNCHER filter and ComponentActivity are left untouched, instead of counting filters.
Deep links: - App.tsx: read the launch URL once. The 300 ms / 1 s re-reads and the per-root ids worked around a second React root per link, which the CLEAR_TOP | SINGLE_TOP patch already prevents: every flow's link now arrives as a `url` event. - deepLink.ts: validate the name against SCENARIO_NAMES instead of casting, so rnvtest://scenario/<typo> stays on the host screen instead of handing an undefined source to the player; reject extra path segments. - fixtures.ts: one fixture() helper; ScenarioName now comes from deepLink.ts, which bun can test without react-native. Controls: - ScenarioScreen: controls are a static list of absolute actions (btn-rate-2, btn-rate-0-5, btn-mute, btn-unmute, btn-loop-on) with logging in one place. Toggles read state at press time, which a deferred Maestro tap gets wrong on iOS. Unused btn-pause, btn-play-pause and scenario-title are gone. Flows use the new ids. Event log: - MARKER_IDS is the single list of markers; EventLogPanel renders from it and keys log lines by a stable id instead of the index. - State is typed rather than cast, reset rebuilds it from one initialState(), handle() is checked for exhaustiveness and notifies subscribers once per event. - An error status no longer overwrites the code a preceding onError reported. - Tests grouped by behaviour, covering thresholds, the error-code order, reset and subscriber notifications.
- e2e/shared/launch-app.yaml: the clean launch and the wait for e2e-host-ready, which ten flows repeated with three versions of the same comment. Its timeout goes from 15 s to 30 s: it waits for the app to start, not for library behaviour, and a freshly cleared app on a busy emulator has taken 15-18 s to get there. - open-scenario.yaml: the "Open in app?" wait and tap only run on iOS, saving 1.5 s per deep link on Android, which never asks. - Every flow that expects playback ends with assertNotVisible: evt-onError, so an error after the asserted marker still fails it (seek, loop, rate, mute-volume and replay-after-end did not check). - Comments point at eventLog.ts, where markers are derived, and the error flows refer to e2e/CONTEXT.md for the cold-launch workaround instead of restating it. - README: document the launch subflow and the evt-onError rule; replace "nothing should wait > 30 s", which the HLS flows already exceeded.
Bugs: - use-rn-version --refresh restored bun.lock with `git checkout`, discarding the uncommitted lockfile of the very package.json change CONTRIBUTING.md says to refresh for. package.json and bun.lock are now written back byte for byte, and every check runs before test-app/node_modules is deleted. - nightly-issue treated any status other than "fail" as green (a typo closed the issue) and posted "Run: undefined" without a run URL; both are now usage errors. - junit-summary read `name` out of `classname`, decoded entities twice (`&#60;` became `<`) and broke astral code points. - record-result accepted missing arguments and unknown row kinds. - matrix-plan quarantined a flow whose comments or steps mentioned "flaky"; it now reads only the top-level tags key, quoted or commented. Design: - Every script's CLI is main({ argv, env, stdout, stderr }) => exit code, so each one is tested in-process, argument validation included. - The matrix plan carries each row's tag selection next to its artifact name. - Removed flowPassRate (never called) and validatePlan's hard-coded 9/6/15 row counts. - Comments cut to the decision and its reason; a shared test helper cleans temporary directories after every test, failing ones included. - android-settle.sh reports when the guest load never dropped.
…ightly - cache-cleanup: a cache family is the key without its last hash only. Stripping every hash collapsed the bun caches of all React Native variants (they differ only in the bun.lock hash) into one and deleted the others every night; against the current cache list the old rule would delete 6 entries (3.3 GB) that are in use. Gradle entries are left to setup-gradle. - e2e.yml / unit.yml: a push to master no longer cancels the previous master run, which writes the caches pull requests restore and checks a merge. - e2e-setup: pin Maestro to 2.10.0, so a Maestro release cannot turn the gate red. - e2e-nightly: legs run the tag selection from the matrix plan instead of a second hard-coded copy; write permissions are scoped to the report job; Android jobs get a readable name. - Bun's version lives in .bun-version, read by setup-bun and the React 19 job. - _e2e-ios: drop the simctl launch warm-up, which the Maestro warm-up flow already does. Stale comments fixed; matrix `blocking` duplicates and a missing timeout-minutes cleaned up.
- e2e-lockfile-refresh: split into `resolve` (read-only: checkout without persisted credentials, setup-bun, `bun install --ignore-scripts`, no node_modules cache) and `open-pr` (contents/pull-requests write, first-party actions only), which commits the lockfiles it receives after checking they are nothing but bun.lock files of versions with an overlay. Before, oven-sh/setup-bun, which takes github.token by default, and freshly resolved dependencies ran in the job that can push. The lockfile resolved with --ignore-scripts is byte-identical to one resolved without. - Pin oven-sh/setup-bun, reactivecircus/android-emulator-runner and gradle/actions/setup-gradle to commit SHAs; dependabot keeps them current. - Install Maestro and actionlint from their release archives, verified by sha256, instead of piping download scripts into bash. - cache-cleanup: closed pull requests are handled on pull_request_target, so caches from fork pull requests (read-only token under pull_request) can be deleted. The job never checks out or runs pull request code. A failed deletion is now reported. - e2e-nightly: check out the e2e-results branch with actions/checkout instead of a git clone with the token in the URL; the main checkout keeps no credentials.
smoke-mute-volume failed on iOS in CI: the tap on btn-mute was sent 264 ms after onEnded, while AVPlayer was still finishing, and reached no button (Maestro reported it done; the app never logged the press). It had passed in the 12 previous iOS runs, with the tap later relative to the end. e2e/shared/wait-for-end.yaml waits for evt-onEnded and then for the screen to stop changing. The five flows that tap after the natural end (seek, hls-seek, loop, mute-volume, replay-after-end) use it. It waits for the UI, it does not retry: a marker the player never produces still fails the flow.
mziolkowskii
marked this pull request as ready for review
September 15, 2026 13:59
- CONTEXT.md: D3, D6 and D7 described plans nothing implements (public streams and DRM in CI, a good-first-test issue catalog, plugin conformance flows); they now state what holds today: local fixtures only, a bug fix brings a reproducing flow, and one flow runs unchanged on both platforms. D4 and D5 no longer comment on billing or recommend a specific authoring tool. - CI_MATRIX_DESIGN.md: describe the macOS concurrency limit without reference to a plan, drop the out-of-scope wish list, and update what changed since: the refresh runs with --ignore-scripts across two jobs, the plan carries tag selections, and RN 0.87 on iOS now runs on hosted runners. - README.md / CONTRIBUTING.md: nightly uses local fixtures like every run; an iOS leg takes about 15 minutes, not 25-35; first-time contributor approval is phrased as the possibility it is.
A workflow skipped by a path filter leaves its checks Pending, which blocks merging once they are required; only a job skipped by `if:` reports Success. Drop `paths-ignore` from the pull_request trigger of unit.yml and e2e.yml (the push trigger keeps it, that is not a gate), and say in the path-filtered lint-workflows and docs-build workflows that they must stay optional. Also stop calling the check names "pinned" while nothing is required yet, and correct the cache-cleanup note: setup-gradle hashes the callee job's matrix, which is empty, so the three Android legs may share one entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ec fix ios/Video-Bridging-Header.h held one `#import <React/RCTViewManager.h>` and was referenced only by the `public_header_files` entry that dc73e72 removed, so nothing loads it any more. The podspec change itself is the one @GratwickEnt proposed in #5085, with the root cause analysed in #5084; the design doc now says so. Co-authored-by: GratwickEnt <178044584+GratwickEnt@users.noreply.github.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The unit job is called "lint, typecheck, test" but the tests it runs were outside both gates: the library's ESLint ignored __tests__/ and its tsconfig only included src, and test-app excluded *.test.ts the same way. Bun strips types at run time, so useManagedInstance.test.ts passed while indexing a function value as a type and passing an option @types/react-test-renderer 18 does not know; its StrictMode branch was also dead (nothing ever passed strict=true), so it is gone and the closing comment explains what is not covered. Each package gets a tsconfig.test.json that adds its tests plus a shared config/bun-test.d.ts shim for the parts of bun:test the suites use. No bun-types: its 1.3.x line peers on @types/react ^19 while the workspace pins ^18, and a new devDependency would mean regenerating both matrix lockfiles. `typecheck` runs the test config after the build one, and ESLint's parserOptions.project includes it in place of the ignore rules. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, pin Bun 1.3.1
- The 404 and broken-manifest flows claimed to assert onError, but the
test app maps onStatusChange('error') onto the same marker, so they
pass while #5083 (onError silent for asynchronous load failures) is
open. The headers and CONTEXT.md now say so and point at the issue.
- The pull request template and e2e/README.md asked for a reproducing
flow and a run on both platforms without an out; a contributor on
Linux cannot run iOS. Both now accept a "Test plan" explanation, since
CI runs both platforms. D6 in CONTEXT.md keeps the strict rule.
- package.json still declared bun@1.1.42 while .bun-version says 1.3.1,
and 1.1.x does not read the committed lockfile format. CONTRIBUTING.md
gets the one-line install of the pinned version.
- CI_MATRIX_DESIGN.md notes that no leg compiles drm-plugin's native code.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Okelm
approved these changes
Sep 16, 2026
github-project-automation
Bot
moved this from To Triage
to Done
in React Native Video - Public Roadmap
mziolkowskii
added a commit
that referenced
this pull request
Sep 17, 2026
master now type-checks and lints __tests__ (#5096), which these tests predate. Extend the bun:test typings with mock.module, resolves, toBeDefined, toBeGreaterThan/toBeLessThan and toThrow(ErrorClass), use the real DrmParams field (licenseUrl) in fixtures, drop two unused @ts-expect-error directives and apply Prettier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mziolkowskii
added a commit
that referenced
this pull request
Sep 17, 2026
master now type-checks and lints __tests__ (#5096), which these tests predate. Extend the bun:test typings with mock.module, resolves, toBeDefined, toBeGreaterThan/toBeLessThan and toThrow(ErrorClass), and apply Prettier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 of 18 tasks
5 of 15 tasks
mziolkowskii
added a commit
that referenced
this pull request
Oct 8, 2026
…5097) * fix(core): stop createSource mutating the caller's VideoConfig createSourceFromVideoConfig wrote its defaults back into the object it was given: initializeOnCreation: true, the platform DRM type and the normalized externalSubtitles. useVideoPlayer keys the player on JSON.stringify(source), so a config the caller keeps around (a module constant, a useMemo/useState value) produced a different key on the next render, and useManagedInstance destroyed and recreated the player. Build the native config as a new object instead. The recreate path is inferred from useVideoPlayer/useManagedInstance, not reproduced in a rendered component; the mutation itself is covered by a test. * test(core): cover source normalization sourceFactory: string/asset/config/VideoPlayerSource inputs, the initializeOnCreation, DRM-type and subtitle defaults, every typed error code, native errors parsed through tryParseNativeVideoError, and a guard that the caller's config is not mutated. react-native and nitro-modules are mocked with mock.module so no native code is involved. * fix(core): propagate native errors reliably through onError and promises - triggerJSEvent returned true for an event whose last listener had been removed (the Set still existed, empty), so throwError treated the error as handled and swallowed it: after unsubscribing from onError, play()/pause()/ seek failures vanished silently. Report false when no listener is registered. - wrapPromise re-threw from inside its own .catch when nobody listened to onError, so the outer promise never settled: await initialize() on a native rejection hung forever and logged an unhandled rejection. With a listener it rejected with undefined. It now always rejects with the parsed VideoRuntimeError and notifies onError listeners first. - release() called twice inside the 5 s grace window released the native player twice; guard with a released flag. * test(core): cover event routing and VideoPlayer error propagation events: every ALL_PLAYER_EVENTS entry routes to the emitter method of the same name (a new event missing from the switch fails here instead of in an app), onError stays JS-only, subscriptions remove, duplicates are deduped, clearAllEvents clears both sides, and trigger reports listener presence. VideoPlayer: sync methods throw or deliver to onError, unparsable errors are rethrown, native promise rejections reject with the parsed error whether or not onError is subscribed, and release is idempotent. The react-native / nitro fakes move to __tests__/helpers/nativeMocks.ts: bun's mock.module is process-wide, so two files registering their own fakes for the same module fought over which one the library saw. * test(core): cover useVideoPlayer Renders the hook with react-test-renderer. The shared native mocks now record every player the fake factory creates and keep emitter listeners, so tests can emit native events and check what was subscribed. * docs: list the core unit tests in the contributing guide * test(core): make the core tests pass tsc and ESLint master now type-checks and lints __tests__ (#5096), which these tests predate. Extend the bun:test typings with mock.module, resolves, toBeDefined, toBeGreaterThan/toBeLessThan and toThrow(ErrorClass), use the real DrmParams field (licenseUrl) in fixtures, drop two unused @ts-expect-error directives and apply Prettier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(core): expect onError's shared native subscription #5135 made onError subscribe once to the native emitter's onError so that asynchronous load and playback failures reach JS. The routing test still asserted that onError never touched the emitter and failed after merging master. It now expects exactly one addOnErrorListener call for any number of JS listeners; videoPlayerEvents.test.ts covers delivery through it. Also drop the `() => void x++` shorthand the tests used for one-liners, which tripped the no-void rule. * docs(core): state that async player methods reject as well as call onError initialize(), preload() and replaceSourceAsync() now always reject with the parsed VideoRuntimeError; with an onError listener attached they deliver it there too. The docs and the agent skill said errors are delivered to onError instead of thrown, which only holds for the synchronous methods. * refactor(core): drop the redundant drm cast in createSourceFromVideoConfig Both branches of the expression are assignable to NativeVideoConfig['drm']; the cast only hid future type errors. * fix(core): share one error path between sync and async methods, keep cancellations out of onError wrapPromise re-implemented throwError's parse-then-notify logic, and replaceSourceAsync() built its source outside the wrapper, so an invalid source rejected the promise without reaching onError while a native rejection did both. Both paths now go through reportError, and the async methods run their native call inside the wrapper so that a synchronous throw (invalid source, released player) is reported the same way. A load superseded by a newer initialize()/replaceSourceAsync() rejects with player/cancelled (source/cancelled on the source side). That is not a failure, so it no longer reaches onError; the promise still rejects so the caller can tell. The two codes are added to the error unions, which did not list them although both platforms emit them. Tests cover cancellation, the synchronous-throw path of replaceSourceAsync and its native rejection; the release test is renamed to what it asserts (the native player is released once), since the player getter keeps serving the native object during the grace window. * fix(core): keep only JS-superseded cancellations away from onError player/cancelled is not only "a newer load replaced this one": both platforms also reject every load with it once the native player has been released, e.g. after replaceSourceAsync(null). Filtering every cancellation hid that case from onError and left the player silently empty. Loads started from JS now carry a generation counter. A cancellation whose load has been superseded by a newer JS load only rejects; a cancellation with no newer load (a released player) is reported through onError like any other failure. Covered by two tests, one per case. Docs and the skill note that the throw/reject rules describe the iOS and Android players; web has its own VideoPlayer and reports through onError with web/* codes. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Bart Widlarz <bartlomiejwidlarz@gmail.com>
mziolkowskii
added a commit
that referenced
this pull request
Oct 8, 2026
…vice> (#5098) * fix(expo-plugins): add the playback service to manifests without <service> withAndroidNotificationControls pushed onto mainApplication.service with optional chaining, so a manifest that had no <service> element yet (the default Expo template) silently got nothing: no VideoPlaybackService, and notification controls did not work. Create the array, skip the service if it is already declared, and make sure the permissions array exists before pushing. writeToPodfile: mergeContents throws when its anchor is missing instead of reporting didMerge: false, so the warn branch was unreachable and prebuild aborted on a Podfile without `platform :ios`. Catch and warn as intended. * test(expo-plugins): cover the config plugins against parsed native files Runs each registered mod the way prebuild does, with a parsed Info.plist, AndroidManifest, gradle.properties or Podfile as modResults: background audio on/off, Picture-in-Picture on .MainActivity, ExoPlayer extension flags with defaults and stale-entry replacement, the playback service and permissions (including a manifest with no <service> element and a repeated run), writeToPodfile anchors for Expo and react-native-test-app, and the plugin composition per prop. * docs: list the Expo config plugin tests in the contributing guide * test(expo-plugins): make the plugin tests pass tsc and ESLint master now type-checks and lints __tests__ (#5096), which these tests predate. Extend the bun:test typings with mock.module, resolves, toBeDefined, toBeGreaterThan/toBeLessThan and toThrow(ErrorClass), and apply Prettier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(expo-plugins): write the foreground service permissions into the manifest The plugin added FOREGROUND_SERVICE and FOREGROUND_SERVICE_MEDIA_PLAYBACK to config.android.permissions from inside its AndroidManifest mod. expo prebuild applies the app's plugins before it registers its built-in ones, and mods run starting from the last registered, so Expo's own permissions mod has already written the manifest when this mod changes the config: the permissions never reached AndroidManifest.xml. Now that the service is added to the default template, an app using showNotificationControls or playInBackground would get the service without the permissions it needs to run in the foreground (Android 9+, and the media playback type on Android 14+). The permissions are now added to the manifest itself with AndroidConfig.Permissions.ensurePermissions, which does not add them twice, and the tests assert the manifest rather than the config. Verified by compiling the manifest mods with @expo/config-plugins 10.1.2 on Expo's bare template, with this plugin registered before and after the permissions mod: the service and both permissions are present exactly once. * fix(expo-plugins): add permissions via withPermissions, keep extension defaults Review follow-ups on this PR: - withAndroidNotificationControls: use AndroidConfig.Permissions.withPermissions at plugin level instead of ensurePermissions inside the manifest mod. It adds the permissions to config.android.permissions when the plugin is applied, so Expo's own permissions mod writes them whenever it runs, registers a manifest mod of its own, and keeps `expo config` truthful; this is also what v6 did. The warning for a manifest without a main application named the wrong element (.MainActivity, copied from the PiP plugin). - withAndroidExtensions: a key left out of a partial object now keeps its documented default (true) instead of switching the extension off, so { useExoplayerHls: false } no longer disables DASH as well. - writeToPodfile: only a missing anchor (ERR_NO_MATCH) is downgraded to a one-line warning naming the anchor; any other error (an unwritable Podfile) still aborts prebuild. - Tests: the config type comes from ConfigPlugin instead of a direct @expo/config-types import the package does not depend on; the test-app Podfile case guards against indexOf returning -1, which made it pass vacuously; new case for a Podfile that cannot be written. * docs(expo): list what the config plugin changes on each platform The page did not say that the plugin always registers the Android playback service and its two foreground permissions, nor what each option writes. enableBackgroundAudio was described as an Android feature; it only edits UIBackgroundModes in Info.plist, Android uses the playback service instead. * test(expo-plugins): cover the prebuild mod order, skip the read-only case as root Registers Expo's own permissions mod after the plugin, as prebuild does, and runs the manifest mod chain: the permissions must reach the manifest although Expo's mod runs first. The read-only Podfile case is skipped under root, which ignores file permissions. The CONTRIBUTING.md sentence is left to #5097, which rewrites the same line; keeping both edits would conflict at merge. * feat(expo-plugins): add enableAndroidPlaybackService to opt out of the playback service The plugin has registered VideoPlaybackService and the FOREGROUND_SERVICE / FOREGROUND_SERVICE_MEDIA_PLAYBACK permissions for every app since #4754 (it only looked otherwise because neither reached the manifest; fixed earlier in this PR). That stays the default: the player starts the service at runtime when playInBackground or showNotificationControls is set, which prebuild cannot know. An app that uses neither can now pass enableAndroidPlaybackService: false to keep the service and permissions out of its manifest, e.g. to avoid the foreground service declaration in the Play Console. Documented on the Expo page and in the agent skill, where the Android background note wrongly pointed at enableBackgroundAudio (iOS-only). --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Bart Widlarz <bartlomiejwidlarz@gmail.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
Adds a deterministic end-to-end test harness for react-native-video (Maestro + a dedicated
react-native-test-apphost app), a CI matrix that runs it across React Native versions and both platforms, and the unit test setup for the library's JS layer. Includes the iOS fix needed to build on React Native 0.87's prebuilt core, so the PR gate can run 0.87.This is the infrastructure part of #5087, split out so it can be reviewed without the library behavior changes. Those follow as stacked PRs:
createSourcemutation, error propagation, doublerelease())Design and rationale:
e2e/CONTEXT.md(decisions D1–D7) ande2e/CI_MATRIX_DESIGN.md(D8–D13, matrix layout, one-time setup).Motivation
The library had no automated E2E coverage and unit tests did not run in CI. Nothing in this PR changes library behavior; the only library changes are the iOS build fix for RN 0.87 and the unit test tooling.
Fixes #5084. Includes the podspec fix from #5085 by @GratwickEnt (closes #5085), plus the three
<React/...>header imports that are also needed under the prebuilt core.Changes
E2E harness
test-app/: RNTA host app driven purely by deep links (rnvtest://scenario/<name>, validated against a fixed list of scenario names). Player events are rendered as text markers with stabletestIDs; flows assert on those, never on pixels or numbers. Every control sets an absolute value (btn-mute/btn-unmute,btn-rate-2,btn-loop-on), so a tap Maestro delivers late on iOS cannot act on stale state.e2e/flows/: 10 smoke flows (mp4 happy path, HLS load/seek, seek, mute/volume, rate, loop, replay after end, 404 and broken-manifest errors). They share three subflows ine2e/shared/: a clean launch that waits until the app can receive a deep link, opening a scenario, and waiting for the UI to settle after the clip ends before tapping.e2e/fixtures/: deterministic media (8 s mp4, tiny HLS VOD, broken manifest) served by a dependency-freenode:httpserver with Range support.e2e/rn-matrix/: overlay + committed lockfile per non-floor RN version (0.82, 0.87);scripts/e2e/use-rn-version.mjsswitches the test app between them or regenerates a variant's lockfile.CI
e2e.yml(PR gate): Android 0.77 + 0.82 + 0.87, iOS 0.87. One macOS job per PR keeps five concurrent PRs inside the 5-job macOS concurrency limit. No path filter onpull_request(nor onunit.yml): a workflow skipped by a path filter leaves a required check Pending forever, so this is what lets the checks become required later. A docs-only PR costs one macOS run. Runs onmasterare not cancelled by the next push, since they write the caches pull requests restore.e2e-nightly.yml: full 3 RN × devices grid (15 jobs) plus quarantine legs forflaky-tagged flows when any exist. Rows, their tag selection and artifact names come from one tested plan (scripts/e2e/matrix-plan.mjs). One deduplicated GitHub issue per failing row; results appended to an orphane2e-resultsbranch with a consecutive-green counter.e2e-lockfile-refresh.yml: weekly bot PR for matrix lockfile drift.unit.yml: lint, typecheck and unit tests on React 18, plus the library's tests and typecheck on React 19 via a non-committed override install. The tests themselves are inside both gates: each package has atsconfig.test.jsonand ESLint parses it, with a smallconfig/bun-test.d.tsshim instead ofbun-types(whose 1.3.x line peers on@types/react19).lint-workflows.yml: actionlint on.github/**.cache-cleanup.yml: deletes a pull request's caches when it closes (fork pull requests included) and keeps one entry per cache family and ref, so every RN variant keeps its ownnode_modulescache.Tokens and third-party code:
GITHUB_TOKEN. Every workflow declares its permissions: read-only, except the nightlyreportjob (history branch, issues), the lockfile refresh'sopen-prjob (push, pull request) andcache-cleanup(actions: write).bun install --ignore-scripts, no cache, no persisted credentials) and hands the lockfiles to the job that pushes, which checks they are nothing but matrix lockfiles.cache-cleanupusespull_request_targetso caches from fork pull requests can be deleted; that job never checks out or runs pull request code..bun-version.iOS build on RN 0.87 (library)
ios/Video-Bridging-Header.has a public header (the change from fix(ios): do not publish the Swift bridging header as a public header #5085), and the header itself, one#importreferenced nowhere, is removed. It led the umbrella header ahead of the Nitrogen headers and loaded moduleReactfirst, so NitroModules' textual include ofjsi/jsi.hfailed under the prebuilt core. No Swift or build setting references the header.<React/...>. Under the prebuilt coreRCTBridge.his not in the Pods header tree; the framework-style form resolves on RN 0.77 too.Unit tests (library)
bun testrunner forpackages/react-native-video/__tests__, withreact-test-rendereras a devDependency.VideoError) anduseManagedInstance. The remaining library tests exercise fixed behavior and ship with those fixes in the stacked PRs.Contributor docs
CONTRIBUTING.md: a "Testing your change" section (unit test vs. Maestro flow, a reproducing flow for E2E-coverable bug fixes, what to do when no test fits, no retries), what CI runs on a pull request, the RN matrix lockfile regeneration after anypackage.jsonchange, and the pre-commit hooks.Nothing enforces tests yet: the new checks are not required in branch protection, and should only become required after a run of consecutive green runs. Only
unitande2emay ever be required;lint-workflowsand the docs build are path-filtered and must stay optional.Follow-ups (not in this PR)
packages/, excludingnitrogen/generated/**; needs one formatting commit first.packages/drm-plugin's native code in CI by adding@react-native-video/drmtotest-app(and regenerating the matrix lockfiles). No leg builds it today.e2e/CONTEXT.md, "Flow-design gotchas").evt-onErrormarker from the status-error path and assertonErroritself in the 404/broken-manifest flows once the fix lands. Today both paths map onto one marker, so the flows pass with the bug open.master, compare the Gradle cache keys of the three Android legs (setup-gradle hashes the callee's empty matrix, so they may share one entry).Platforms affected
Type of change
Test plan
bun run test: 166 tests pass (library 11; harness 155: CI scripts 110, test app 36, fixture server 9).bun typecheckandbun lintpass and now cover the unit tests too; actionlint passes. Thereact-19job's combination (react and react-test-renderer 19.2.8, @types/react 19.2.18) was reproduced locally: the library's 11 tests, typecheck and lint pass there as well.e2e/shared/wait-for-end.yaml,smoke-mute-volumefailed once when its tap was sent while AVPlayer was still finishing the clip.--ignore-scriptsis byte-identical to one resolved without.master: the nightly, the weekly lockfile refresh and the daily cache prune. The prune rule was simulated against the repository's current cache list.One-time setup
e2e-resultsbranch created (history for the nightly consecutive-green counter).e2e-nightlylabel created (used by the nightly issue deduplication).Checklist
bun run test,bun lintandbun typecheckpass locally.skills/react-native-video/) to match.