Skip to content

Commit 3da2989

Browse files
adamzieladamziel
authored
[CI] Explore fixing two notoriously flaky tests (#2935)
## Motivation for the change, related issues Cleans up resources in two, notoriously flaky unit tests to make them more reliable: * Cleans up resources, calls `php.exit()` * Stops the server to free up the port * Updates the Xdebug DBGP integration test to use an ephemeral port * Adds a handshake timeout and improved error handling for the DBGP server * Ensures the tests fails gracefully when the handshake is not completed in time. The tests we're trying to fix: ``` ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯ FAIL src/test/php-dynamic-loading.spec.ts > PHP 8.0 > XDebug > communicates with default DBGP port FAIL src/test/php-networking.spec.ts > PHP 8.0 > cURL > should support multi handle requests Error: Test timed out in 20000ms. ``` ## Testing Instructions (or ideally a Blueprint) CI :)
1 parent b2d5835 commit 3da2989

File tree

2 files changed

+108
-41
lines changed

2 files changed

+108
-41
lines changed

‎packages/php-wasm/node/src/test/php-dynamic-loading.spec.ts‎

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
import { loadNodeRuntime } from '..';
2-
import { PHP, SupportedPHPVersions } from '@php-wasm/universal';
2+
import {
3+
PHP,
4+
SupportedPHPVersions,
5+
setPhpIniEntries,
6+
} from '@php-wasm/universal';
37
import fs from 'fs';
4-
import { createServer } from 'net';
8+
import { createServer, type AddressInfo } from 'net';
59

610
const phpVersions =
711
'PHP' in process.env ? [process.env['PHP']!] : SupportedPHPVersions;
@@ -66,8 +70,14 @@ describe.each(phpVersions)('PHP %s', async (phpVersion) => {
6670
});
6771

6872
it(
69-
'communicates with default DBGP port',
73+
'communicates with a DBGP client',
7074
async () => {
75+
/**
76+
* Use an ephemeral port to avoid collisions with any stray/debugger
77+
* processes that may already be listening on the default 9003.
78+
* We still verify the full DBGP handshake, only the port is made
79+
* deterministic per-test to remove flakiness.
80+
*/
7181
const queries = [
7282
'feature_set -i 1 -n resolved_breakpoints -v 1',
7383
'feature_set -i 2 -n notify_ok -v 1',
@@ -84,37 +94,76 @@ describe.each(phpVersions)('PHP %s', async (phpVersion) => {
8494

8595
const server = createServer();
8696

87-
server.on('connection', (tcpSource) => {
88-
tcpSource.on('data', (data) => {
89-
if (queries[i]) {
90-
responses += new TextDecoder().decode(data);
91-
const payload = `${Buffer.byteLength(
92-
queries[i]
93-
)}\x00${queries[i]}\x00`;
94-
tcpSource.write(new TextEncoder().encode(payload));
95-
i++;
96-
} else {
97-
stopped = true;
98-
server.close();
99-
}
97+
try {
98+
// Start the server on an available port.
99+
await new Promise<void>((resolve, reject) => {
100+
server.once('error', reject);
101+
server.listen(0, '127.0.0.1', resolve);
100102
});
101-
});
103+
const { port } = server.address() as AddressInfo;
102104

103-
server.listen(9003);
104-
105-
const result = await php.runStream({
106-
code: `<?php
107-
echo "Hello Xdebug World";`,
108-
});
105+
// Point Xdebug at the chosen port/host.
106+
await setPhpIniEntries(php, {
107+
'xdebug.client_port': port,
108+
'xdebug.client_host': '127.0.0.1',
109+
});
109110

110-
await vi.waitUntil(() => stopped, { timeout: 5000 });
111+
const handshake = new Promise<void>((resolve, reject) => {
112+
const timeout = setTimeout(() => {
113+
reject(
114+
new Error(
115+
'Xdebug did not complete the DBGP handshake in time'
116+
)
117+
);
118+
}, 8000);
119+
120+
server.on('connection', (tcpSource) => {
121+
tcpSource.on('data', (data) => {
122+
if (queries[i]) {
123+
responses += new TextDecoder().decode(data);
124+
const payload = `${Buffer.byteLength(
125+
queries[i]
126+
)}\x00${queries[i]}\x00`;
127+
tcpSource.write(
128+
new TextEncoder().encode(payload)
129+
);
130+
i++;
131+
} else if (!stopped) {
132+
stopped = true;
133+
clearTimeout(timeout);
134+
resolve();
135+
}
136+
});
137+
});
138+
139+
server.once('error', (err) => {
140+
clearTimeout(timeout);
141+
reject(err);
142+
});
143+
server.once('close', () => clearTimeout(timeout));
144+
});
111145

112-
expect(responses).toContain(
113-
'<init xmlns="urn:debugger_protocol_v1"'
114-
);
115-
expect(responses).toContain('success="1"></response>');
146+
const result = await php.runStream({
147+
code: `<?php
148+
echo "Hello Xdebug World";`,
149+
});
116150

117-
expect(await result.stdoutText).toEqual('Hello Xdebug World');
151+
await handshake;
152+
153+
expect(responses).toContain(
154+
'<init xmlns="urn:debugger_protocol_v1"'
155+
);
156+
expect(responses).toContain('success="1"></response>');
157+
158+
expect(await result.stdoutText).toEqual(
159+
'Hello Xdebug World'
160+
);
161+
} finally {
162+
// Ensure the port is always freed for subsequent PHP versions.
163+
if (server.listening) {
164+
await new Promise((resolve) => server.close(resolve));
165+
}
166+
}
118167
},
119168
{ timeout: 20_000 }
120169
);

‎packages/php-wasm/node/src/test/php-networking.spec.ts‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
4949

5050
phpLoaderOptions.forEach((options) => {
5151
it('should be able to make a request to a server', async () => {
52+
let php: PHP | undefined;
5253
try {
5354
const serverUrl = await startServer();
54-
const php = new PHP(await loadNodeRuntime(phpVersion, options));
55+
php = new PHP(await loadNodeRuntime(phpVersion, options));
5556
await setPhpIniEntries(php, {
5657
allow_url_fopen: 1,
5758
disable_functions: '',
@@ -65,17 +66,18 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
6566
const { text } = await php.run({
6667
scriptPath: '/tmp/test.php',
6768
});
68-
php.exit();
6969
expect(text).toEqual('response from express');
7070
} finally {
71+
php?.exit();
7172
await stopServer(server);
7273
}
7374
});
7475

7576
it('should support fopen() and fread() until EOF', async () => {
77+
let php: PHP | undefined;
7678
try {
7779
const serverUrl = await startServer();
78-
const php = new PHP(await loadNodeRuntime(phpVersion, options));
80+
php = new PHP(await loadNodeRuntime(phpVersion, options));
7981
await setPhpIniEntries(php, {
8082
allow_url_fopen: 1,
8183
disable_functions: '',
@@ -141,20 +143,19 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
141143
const { text } = await php.run({
142144
scriptPath: '/tmp/test.php',
143145
});
144-
php.exit();
145146
expect(text).toContain('Stream select result: 1');
146147
} finally {
148+
php?.exit();
147149
await stopServer(server);
148150
}
149151
}, 10000);
150152

151153
describe('cURL', () => {
152154
it('should support single handle requests', async () => {
155+
let php: PHP | undefined;
153156
try {
154157
const serverUrl = await startServer();
155-
const php = new PHP(
156-
await loadNodeRuntime(phpVersion, options)
157-
);
158+
php = new PHP(await loadNodeRuntime(phpVersion, options));
158159
await setPhpIniEntries(php, {
159160
allow_url_fopen: 1,
160161
disable_functions: '',
@@ -173,19 +174,20 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
173174
const { text } = await php.run({
174175
scriptPath: '/tmp/test.php',
175176
});
176-
php.exit();
177177
expect(text).toEqual('response from express');
178178
} finally {
179+
php?.exit();
179180
await stopServer(server);
180181
}
181182
});
182183

183184
it(
184185
'should support multi handle requests',
185186
async () => {
187+
let php: PHP | undefined;
186188
try {
187189
const serverUrl = await startServer();
188-
const php = new PHP(
190+
php = new PHP(
189191
await loadNodeRuntime(phpVersion, options)
190192
);
191193
await setPhpIniEntries(php, {
@@ -206,9 +208,21 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
206208
$mh = curl_multi_init();
207209
curl_multi_add_handle($mh, $ch1);
208210
curl_multi_add_handle($mh, $ch2);
211+
$started = microtime(true);
209212
do {
210-
curl_multi_exec($mh, $running);
211-
curl_multi_select($mh);
213+
$status = curl_multi_exec($mh, $running);
214+
if ($status !== CURLM_OK && $status !== CURLM_CALL_MULTI_PERFORM) {
215+
throw new Exception('curl_multi_exec failed: '.curl_multi_strerror($status));
216+
}
217+
if ($running > 0) {
218+
$rc = curl_multi_select($mh, 1.0);
219+
if ($rc === -1) {
220+
usleep(100000);
221+
}
222+
}
223+
if ((microtime(true) - $started) > 5) {
224+
throw new Exception('curl_multi_exec timed out');
225+
}
212226
} while ($running > 0);
213227
echo curl_multi_getcontent($ch1)."\\n";
214228
echo curl_multi_getcontent($ch2);
@@ -222,11 +236,15 @@ describe.each(phpVersions)('PHP %s', (phpVersion) => {
222236
const { text } = await php.run({
223237
scriptPath: '/tmp/test.php',
224238
});
225-
php.exit();
226239
expect(text).toEqual(
227240
'response from express\nresponse from express'
228241
);
229242
} finally {
243+
try {
244+
php?.exit();
245+
} catch {
246+
// ignore cleanup failures
247+
}
230248
await stopServer(server);
231249
}
232250
},

0 commit comments

Comments
 (0)