Repository navigation
chore(🐙): keep e2e runs on the app's socket and fail an image check without a reference on CI - #549
Open
YevheniiKotyrlo wants to merge 4 commits into
Open
YevheniiKotyrlo wants to merge 4 commits into
YevheniiKotyrlo wants to merge 4 commits into
Conversation
globalSetup made every socket the test server accepted the tests' client before it said hello, and a test file keeps the first value a global had when it read it, so a connection that never says hello takes over every test file that starts after it: each eval waits out its 30 s. The client is now the socket that said hello last, test files read it through an object set up before any of them load, and teardown closes every socket the server accepted and removes the adb port mapping setup made. An eval reads the client once, before arming its timeout, so a hello arriving while it waits no longer moves its timeout to the new socket, and an eval with no client rejects without leaving a timer behind.
checkImage wrote a missing reference and passed, so on CI a check with no stored image compared nothing. On CI it now fails and writes the image as <reference>.test.png, where the workflows already upload failed checks. Outside CI it still writes the reference, which is how one is made.
Contributor
Author
|
The three new specs against
With this change: 9 passed. The whole suite through CI's Android e2e step on an Android 15 emulator (API 35), Metro and jest with |
This branch has not been deployed
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.
Two fixes to the e2e harness, so that a red run means a failing test and a green one means every check compared something.
Problem
globalSetup.tsmakes every socket the test server acceptsglobal.testClientbefore the socket says hello. The example app opens a new socket on every retry, and a test file keeps the first value a global had when it read it (jest-environment-nodeinstalls a lazy getter per global), so a connection that never says hello takes over every test file that starts after it: each eval is sent to it and waits out its 30 s. Teardown then closes only that last socket and leaves theadb reversemapping on the device.RemoteTestingClient.handleResponsealso readsthis.clientagain when its timeout fires, so a hello arriving while an eval waits takes the listener off the new socket and leaves it on the old one, and an eval with no client arms its timeout before it throws.checkImagewrites a reference it does not find and returns 0, so on CI a check without a stored image compares nothing and passes, and the image it drew only reaches the uploaded artifact. Every check onmainhas its reference today, so this is about the next check that lands without one.Solution
The client is the socket that said hello last, held in
global.testConnection, an objectglobalSetupcreates before any test file loads, so every file reads the current client through it. Teardown closes every socket the server accepted and both servers, and removes the port mapping setup made. An eval reads its socket once, before arming its timeout.On CI, a check without a reference fails and writes its image as
<reference>.test.png, the name a failed comparison already uses and the workflows already upload. Outside CI it still writes the reference, which is how one is made;overwritestill writes it everywhere.config.tsreadsCIwith the suite's other flags.These are wcandillon/react-native-skia#4240 and #4231 for this repo.
Tests
globalSetup.spec.tsdrives the real server on a port of its own: an eval goes to the app that said hello rather than to a connection that never did, the client moves to the socket that says hello next, and teardown closes every socket and removes the mapping.RemoteTestingClient.spec.tsdrives an eval over fake sockets: it times out on the socket it was sent to when another app says hello meanwhile, and rejects without arming a timer when no app has said hello.checkImage.spec.ts: on CI a check without a reference fails and leaves<reference>.test.png, outside CI it writes the reference, and on CI a present reference is compared and leaves no.test.png.Against
main's harness six of the nine fail: the eval reaches the silent socket, the app's socket stays open, nothing removes the mapping, the listener stays on the first socket, an eval with no client throws aTypeErrorinstead of its own error, and the check without a reference passes on CI. With the change all nine pass.Verification
yarn lint·yarn tsc·yarn turbo run build --filter='./packages/*'· the Expo template's web build ·yarn test:plugin·yarn check-dawn·yarn build:docs. CI's Android e2e step (debug build, Metro and jest withCI=true) on an Android 15 emulator (API 35): 45 of 47 suites and 228 of 230 tests passed, with no check failing for a missing reference and no image written. The two failures, the YUV cases ofExternalTextureColorSpaceandNativeBufferWrapper, fail on that emulator withmaintoo (219 of 221 there; they render on the host's GPU). On devices the whole suite passes, 230 of 230: on a Galaxy S23 (Android 14), and on an iPhone 12 Pro Max (iOS 27.0.1) built and run as CI's iOS job does on a simulator (Debug build, Metro withCI=true).mainpasses 221 of 221 on both. Not run here: the semantic-release dry run, which needs the repository's token.