Skip to content

fix(🐙): fix e2e test flakiness #3299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(🐙): fix e2e test flakiness #3299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/example/ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2193,7 +2193,7 @@ SPEC CHECKSUMS:
React-Mapbuffer: 0502faf46cab8fb89cfc7bf3e6c6109b6ef9b5de
React-microtasksnativemodule: 663bc64e3a96c5fc91081923ae7481adc1359a78
react-native-safe-area-context: 286b3e7b5589795bb85ffc38faf4c0706c48a092
react-native-skia: 9c69b3dd2c957ddc31d9f5efcf3a6320e0b55461
react-native-skia: 36463ca61a318c139330095cc2314d72088aea4c
react-native-slider: 27263d134d55db948a4706f1e47d0ec88fb354dd
React-NativeModulesApple: 16fbd5b040ff6c492dacc361d49e63cba7a6a7a1
React-perflogger: ab51b7592532a0ea45bf6eed7e6cae14a368b678
Expand Down
93 changes: 67 additions & 26 deletions apps/example/src/Tests/Tests.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,43 @@
assets: { [key: string]: any };
}

interface TestRequest {
id: string;
body: string;
}

interface TestResponse {
id: string;
data: string | ArrayBuffer;
error?: string;
}

export const Tests = ({ assets }: TestsProps) => {
const viewRef = useRef<View>(null);
const ref = useCanvasRef();
const [client, hostname] = useClient();
const [drawing, setDrawing] = useState<any>(null);
const [screen, setScreen] = useState<any>(null);
const [currentRequestId, setCurrentRequestId] = useState<string | null>(null);

const sendResponse = (id: string, data: string | ArrayBuffer, error?: string) => {
if (client) {
const response: TestResponse = { id, data, error };
client.send(JSON.stringify(response));
}
};

useEffect(() => {
if (client !== null) {
client.onmessage = (e) => {
const tree: any = JSON.parse(e.data);
if (tree.code) {
client.send(
JSON.stringify(
eval(
try {
const request: TestRequest = JSON.parse(e.data);
const tree: any = JSON.parse(request.body);
setCurrentRequestId(request.id);

if (tree.code) {
try {
const result = eval(
`(function Main() {
return (${tree.code})(this.Skia, this.ctx, this.size, this.scale);
})`
Expand All @@ -48,18 +71,28 @@
ctx: parseProps(tree.ctx, assets),
size: size * PixelRatio.get(),
scale: s,
})
)
);
} else if (typeof tree.screen === "string") {
const Screen = Screens[tree.screen];
if (!Screen) {
throw new Error(`Unknown screen: ${tree.screen}`);
});
sendResponse(request.id, JSON.stringify(result));
} catch (error) {
sendResponse(request.id, "", `Code execution error: ${error}`);
}
} else if (typeof tree.screen === "string") {
const Screen = Screens[tree.screen];
if (!Screen) {
sendResponse(request.id, "", `Unknown screen: ${tree.screen}`);
return;
}
setScreen(React.createElement(Screen));
} else {
try {
const node = parseNode(tree, assets);
setDrawing(node as SerializedNode);
} catch (error) {
sendResponse(request.id, "", `Node parsing error: ${error}`);
}
}
setScreen(React.createElement(Screen));
} else {
const node = parseNode(tree, assets);
setDrawing(node as SerializedNode);
} catch (error) {
console.error("Error processing request:", error);
}
};
return () => {
Expand All @@ -69,8 +102,9 @@
return;
}, [assets, client]);
useEffect(() => {
if (drawing) {
if (drawing && currentRequestId) {
const requestId = currentRequestId;
const it = setTimeout(() => {

Check failure on line 107 in apps/example/src/Tests/Tests.tsx

View workflow job for this annotation

GitHub Actions / lint 

React Hook useEffect has a missing dependency: 'sendResponse'. Either include it or remove the dependency array
if (ref.current) {
ref.current
.makeImageSnapshotAsync({
Expand All @@ -82,11 +116,12 @@
.then((image) => {
if (image && client) {
const data = image.encodeToBytes();
client.send(data);
const base64Data = btoa(String.fromCharCode(...data));
sendResponse(requestId, base64Data);
}
})
.catch((e) => {
console.error(e);
sendResponse(requestId, "", `Drawing error: ${e}`);
});
}
}, timeToDraw);
Expand All @@ -95,26 +130,32 @@
};
}
return;
}, [client, drawing, ref]);
}, [client, drawing, ref, currentRequestId]);
useEffect(() => {
if (screen) {
if (screen && currentRequestId) {
const requestId = currentRequestId;
const it = setTimeout(async () => {

Check failure on line 137 in apps/example/src/Tests/Tests.tsx

View workflow job for this annotation

GitHub Actions / lint 

React Hook useEffect has a missing dependency: 'sendResponse'. Either include it or remove the dependency array
const image = await makeImageFromView(viewRef as RefObject<View>);
if (image && client) {
const data = image.encodeToBytes();
client.send(data);
try {
const image = await makeImageFromView(viewRef as RefObject<View>);
if (image && client) {
const data = image.encodeToBytes();
const base64Data = btoa(String.fromCharCode(...data));
sendResponse(requestId, base64Data);
}
} catch (e) {
sendResponse(requestId, "", `Screen capture error: ${e}`);
}
}, timeToDraw);
return () => {
clearTimeout(it);
};
}
return;
}, [client, screen]);
}, [client, screen, currentRequestId]);
return (
<View style={{ flex: 1, backgroundColor: "white" }}>
<Text style={{ color: "black" }}>
{client === null

Check failure on line 158 in apps/example/src/Tests/Tests.tsx

View workflow job for this annotation

GitHub Actions / lint 

React Hook useEffect has a missing dependency: 'sendResponse'. Either include it or remove the dependency array
? `⚪️ Connecting to ${hostname}. Use yarn e2e to run tests.`
: "🟢 Waiting for the server to send tests"}
</Text>
Expand Down
101 changes: 92 additions & 9 deletions packages/skia/src/renderer/__tests__/setup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ beforeAll(async () => {
assets.set(skiaLogoJpeg, "skiaLogoJpeg");
});

afterAll(() => {
if (surface && E2E && 'destroy' in surface) {
(surface as RemoteSurface).destroy();
}
});

export const wait = (ms: number) =>
new Promise((resolve) => setTimeout(resolve, ms));

Expand Down Expand Up @@ -429,20 +435,35 @@ class LocalSurface implements TestingSurface {
}
}

interface TestRequest {
id: string;
body: string;
}

interface TestResponse {
id: string;
data: string | Buffer;
error?: string;
}

class RemoteSurface implements TestingSurface {
readonly width = 256;
readonly height = 256;
readonly fontSize = 32;
readonly OS = global.testOS;
readonly arch = global.testArch;

private pendingRequests = new Map<string, { resolve: (value: any) => void; reject: (error: Error) => void; isJson: boolean; timeout: NodeJS.Timeout }>();
private messageListenerAttached = false;
private isDestroyed = false;

eval<Ctx extends EvalContext, R>(
fn: (Skia: Skia, ctx: Ctx) => any,
context?: Ctx
): Promise<R> {
const ctx = this.prepareContext(context);
const body = { code: fn.toString(), ctx };
return this.handleImageResponse<R>(JSON.stringify(body), true);
return this.sendRequest<R>(JSON.stringify(body), true);
}

async drawOffscreen<Ctx extends EvalContext>(
Expand All @@ -464,7 +485,7 @@ surface.flush();
return surface.makeImageSnapshot().encodeToBase64();
}`;
const body = { code, ctx };
const base64 = await this.handleImageResponse<string>(
const base64 = await this.sendRequest<string>(
JSON.stringify(body),
true
);
Expand All @@ -478,11 +499,11 @@ return surface.makeImageSnapshot().encodeToBase64();
}

async draw(node: ReactNode) {
return this.handleImageResponse(await serialize(node));
return this.sendRequest(await serialize(node));
}

screen(screen: string) {
return this.handleImageResponse(JSON.stringify({ screen }));
return this.sendRequest(JSON.stringify({ screen }));
}

private get client() {
Expand All @@ -502,16 +523,78 @@ return surface.makeImageSnapshot().encodeToBase64();
return ctx;
}

private handleImageResponse<R = SkImage>(
private sendRequest<R = SkImage>(
body: string,
json?: boolean
): Promise<R> {
return new Promise((resolve) => {
this.client.once("message", (raw: Buffer) => {
resolve(json ? JSON.parse(raw.toString()) : this.decodeImage(raw));
const requestId = this.generateRequestId();
this.ensureMessageListener();

return new Promise((resolve, reject) => {
const timeout = setTimeout(() => {
this.pendingRequests.delete(requestId);
reject(new Error(`Request ${requestId} timed out after 180 seconds`));
}, 180000);

this.pendingRequests.set(requestId, {
resolve,
reject,
isJson: json || false,
timeout
});

const request: TestRequest = { id: requestId, body };
this.client.send(JSON.stringify(request));
});
}

private ensureMessageListener() {
if (!this.messageListenerAttached && this.client) {
this.client.on("message", (raw: Buffer) => {
if (this.isDestroyed) {
return;
}

try {
const response: TestResponse = JSON.parse(raw.toString());
const pending = this.pendingRequests.get(response.id);

if (!pending) {
// Silently ignore responses for requests that have already timed out or completed
return;
}

clearTimeout(pending.timeout);
this.pendingRequests.delete(response.id);

if (response.error) {
pending.reject(new Error(response.error));
} else {
const result = pending.isJson ? JSON.parse(response.data as string) : this.decodeImage(Buffer.from(response.data as string, 'base64'));
pending.resolve(result);
}
} catch (error) {
if (!this.isDestroyed) {
console.error("Error processing response:", error);
}
}
});
this.client.send(body);
this.messageListenerAttached = true;
}
}

destroy() {
this.isDestroyed = true;
// Clean up any pending requests
this.pendingRequests.forEach(({ timeout, reject }) => {
clearTimeout(timeout);
reject(new Error("RemoteSurface destroyed"));
});
this.pendingRequests.clear();
}

private generateRequestId(): string {
return `req_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;
}

private decodeImage(raw: Buffer) {
Expand Down
Loading