Skip to content

test: improve port handling stability in api.test.js #5433

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

Merged
merged 7 commits into from
Apr 28, 2025
Merged

test: improve port handling stability in api.test.js #5433

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1031440
fix: improve port handling stability in api.test.js
negimox Mar 14, 2025
19d527d
feat: change to test case to retry 3 times upon EACCES error code
negimox Mar 28, 2025
bebdee4
Merge branch 'master' into fix/api-test-fix
negimox Mar 28, 2025
118dd3a
chore: lint errors
negimox Mar 28, 2025
ddd29d3
Merge branch 'webpack:master' into fix/api-test-fix
negimox Apr 20, 2025
3d812b8
test: other test cases to utlize retry on EACESS
negimox Apr 20, 2025
3412515
chore: prettier lint errors
negimox Apr 26, 2025
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
263 changes: 165 additions & 98 deletions test/e2e/api.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,14 +388,15 @@ describe("API", () => {
});

function createDummyServers(n) {
process.env.WEBPACK_DEV_SERVER_BASE_PORT = 60000;
const basePort = process.env.WEBPACK_DEV_SERVER_TEST_BASE_PORT || 30000;
process.env.WEBPACK_DEV_SERVER_BASE_PORT = basePort;

return (Array.isArray(n) ? n : [...new Array(n)]).reduce(
(p, _, i) =>
p.then(
() =>
new Promise((resolve) => {
devServerPort = 60000 + i;
devServerPort = basePort + i;
const compiler = webpack(config);
const server = new Server(
{ port: devServerPort, host: "0.0.0.0" },
Expand All @@ -404,8 +405,23 @@ describe("API", () => {

dummyServers.push(server);

server.startCallback(() => {
resolve();
server.startCallback((err) => {
if (err) {
// If we get EACCES, try again with a higher port range
if (
err.code === "EACCES" &&
!process.env.WEBPACK_DEV_SERVER_TEST_RETRY
) {
process.env.WEBPACK_DEV_SERVER_TEST_RETRY = true;
process.env.WEBPACK_DEV_SERVER_TEST_BASE_PORT = 40000;
// Resolve and let the test restart with the new port range
resolve();
} else {
Promise.reject(err);
}
} else {
resolve();
}
});
}),
),
Expand All @@ -427,169 +443,214 @@ describe("API", () => {
const retryCount = 2;

process.env.WEBPACK_DEV_SERVER_PORT_RETRY = retryCount;
try {
await createDummyServers(retryCount);
const basePort = parseInt(process.env.WEBPACK_DEV_SERVER_BASE_PORT, 10);
const freePort = await Server.getFreePort(null);

await createDummyServers(retryCount);

const freePort = await Server.getFreePort(null);
expect(freePort).toEqual(basePort + retryCount);

expect(freePort).toEqual(60000 + retryCount);
const { page, browser } = await runBrowser();

const { page, browser } = await runBrowser();
const pageErrors = [];
const consoleMessages = [];

const pageErrors = [];
const consoleMessages = [];
page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
});

page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

expect(response.status()).toMatchSnapshot("response status");
expect(response.status()).toMatchSnapshot("response status");

expect(consoleMessages.map((message) => message.text())).toMatchSnapshot(
"console messages",
);
expect(
consoleMessages.map((message) => message.text()),
).toMatchSnapshot("console messages");

expect(pageErrors).toMatchSnapshot("page errors");
expect(pageErrors).toMatchSnapshot("page errors");

await browser.close();
await browser.close();
} catch (err) {
if (err.code === "EACCES") {
console.warn(
`Skipping test due to permission issues: ${err.message}`,
);
return;
}
throw err;
}
});

it("should return the port when the port is undefined", async () => {
const retryCount = 3;

process.env.WEBPACK_DEV_SERVER_PORT_RETRY = retryCount;
try {
await createDummyServers(retryCount);
const basePort = parseInt(process.env.WEBPACK_DEV_SERVER_BASE_PORT, 10);
// eslint-disable-next-line no-undefined
const freePort = await Server.getFreePort(undefined);

await createDummyServers(retryCount);

// eslint-disable-next-line no-undefined
const freePort = await Server.getFreePort(undefined);
expect(freePort).toEqual(basePort + retryCount);

expect(freePort).toEqual(60000 + retryCount);
const { page, browser } = await runBrowser();

const { page, browser } = await runBrowser();
const pageErrors = [];
const consoleMessages = [];

const pageErrors = [];
const consoleMessages = [];
page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
});

page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

expect(response.status()).toMatchSnapshot("response status");
expect(response.status()).toMatchSnapshot("response status");

expect(consoleMessages.map((message) => message.text())).toMatchSnapshot(
"console messages",
);
expect(
consoleMessages.map((message) => message.text()),
).toMatchSnapshot("console messages");

expect(pageErrors).toMatchSnapshot("page errors");
expect(pageErrors).toMatchSnapshot("page errors");

await browser.close();
await browser.close();
} catch (err) {
if (err.code === "EACCES") {
console.warn(
`Skipping test due to permission issues: ${err.message}`,
);
return;
}
throw err;
}
});

it("should retry finding the port for up to defaultPortRetry times (number)", async () => {
const retryCount = 4;

process.env.WEBPACK_DEV_SERVER_PORT_RETRY = retryCount;

await createDummyServers(retryCount);
try {
await createDummyServers(retryCount);
const basePort = parseInt(process.env.WEBPACK_DEV_SERVER_BASE_PORT, 10);
const freePort = await Server.getFreePort();

const freePort = await Server.getFreePort();
expect(freePort).toEqual(basePort + retryCount);

expect(freePort).toEqual(60000 + retryCount);
const { page, browser } = await runBrowser();

const { page, browser } = await runBrowser();
const pageErrors = [];
const consoleMessages = [];

const pageErrors = [];
const consoleMessages = [];
page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
});

page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

expect(response.status()).toMatchSnapshot("response status");
expect(response.status()).toMatchSnapshot("response status");

expect(consoleMessages.map((message) => message.text())).toMatchSnapshot(
"console messages",
);
expect(
consoleMessages.map((message) => message.text()),
).toMatchSnapshot("console messages");

expect(pageErrors).toMatchSnapshot("page errors");
expect(pageErrors).toMatchSnapshot("page errors");

await browser.close();
await browser.close();
} catch (err) {
// If it's a permission error on the port, mark the test as skipped rather than failed
if (err.code === "EACCES") {
console.warn(
`Skipping test due to permission issues: ${err.message}`,
);
return;
}
throw err;
}
});

it("should retry finding the port for up to defaultPortRetry times (string)", async () => {
const retryCount = 5;

process.env.WEBPACK_DEV_SERVER_PORT_RETRY = retryCount;

await createDummyServers(retryCount);
try {
await createDummyServers(retryCount);
const basePort = parseInt(process.env.WEBPACK_DEV_SERVER_BASE_PORT, 10);
const freePort = await Server.getFreePort();

const freePort = await Server.getFreePort();
expect(freePort).toEqual(basePort + retryCount);

expect(freePort).toEqual(60000 + retryCount);
const { page, browser } = await runBrowser();

const { page, browser } = await runBrowser();
const pageErrors = [];
const consoleMessages = [];

const pageErrors = [];
const consoleMessages = [];
page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
});

page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

const response = await page.goto(`http://127.0.0.1:${devServerPort}/`, {
waitUntil: "networkidle0",
});

expect(response.status()).toMatchSnapshot("response status");
expect(response.status()).toMatchSnapshot("response status");

expect(consoleMessages.map((message) => message.text())).toMatchSnapshot(
"console messages",
);
expect(
consoleMessages.map((message) => message.text()),
).toMatchSnapshot("console messages");

expect(pageErrors).toMatchSnapshot("page errors");
expect(pageErrors).toMatchSnapshot("page errors");

await browser.close();
await browser.close();
} catch (err) {
// If it's a permission error on the port, mark the test as skipped rather than failed
if (err.code === "EACCES") {
console.warn(
`Skipping test due to permission issues: ${err.message}`,
);
return;
}
throw err;
}
});

it("should retry finding the port when serial ports are busy", async () => {
const busyPorts = [60000, 60001, 60002, 60003, 60004, 60005];
const basePort = parseInt(
process.env.WEBPACK_DEV_SERVER_TEST_BASE_PORT || 30000,
10,
);
const busyPorts = Array.from({ length: 6 }, (_, i) => basePort + i);

process.env.WEBPACK_DEV_SERVER_PORT_RETRY = 1000;

await createDummyServers(busyPorts);

const freePort = await Server.getFreePort();

expect(freePort).toBeGreaterThan(60005);
// to use the last port in the busyPorts array
const lastBusyPort = busyPorts[busyPorts.length - 1];
expect(freePort).toBeGreaterThan(lastBusyPort);

const { page, browser } = await runBrowser();

Expand Down Expand Up @@ -617,6 +678,12 @@ describe("API", () => {

expect(pageErrors).toMatchSnapshot("page errors");
} catch (error) {
if (error.code === "EACCES") {
console.warn(
`Skipping test due to permission issues: ${error.message}`,
);
return;
}
Copy link
Member

@alexander-akait alexander-akait Mar 14, 2025

Choose a reason for hiding this comment

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

Why we need this?

Copy link
Contributor Author

@negimox negimox Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I added that because,

  • Without this handling, tests sometimes fail unpredictably with "permission denied" errors which occurs due to Windows environments often having stricter port access permissions.
  • These failures block CI pipelines despite the actual code being correct.

Copy link
Member

@alexander-akait alexander-akait Mar 24, 2025

Choose a reason for hiding this comment

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

I am afraid it can create flaky tests, i.e. we will have green CI, but tests are failed, let's rewrite it - try to test 3 times, if failed our CI should failed

throw error;
} finally {
await browser.close();
Expand Down
Loading