-
Notifications
You must be signed in to change notification settings - Fork 105
fix: nitro app workaround #363
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
||
| - name: Run E2E Tests | ||
| run: cd workbench/${{ matrix.app.name }} && pnpm dev & echo "starting tests in 10 seconds" && sleep 10 && pnpm vitest run packages/core/e2e/dev.test.ts && pnpm run test:e2e | ||
| run: cd workbench/${{ matrix.app.name }} && pnpm dev & echo "starting tests in 10 seconds" && sleep 10 && pnpm vitest run packages/core/e2e/dev.test.ts && sleep 5 && pnpm run test:e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
race condition between nitro apps rebuilding after dev test cleanup and e2e tests starting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔧 Build Fix:
The module imports exec, promisify, and declares execAsync but never uses it. The function only uses execFileSync, making the promisified version of exec redundant and causing a TypeScript error.
View Details
📝 Patch Details
diff --git a/packages/utils/src/get-port.ts b/packages/utils/src/get-port.ts
index 69d548c..f61e8ac 100644
--- a/packages/utils/src/get-port.ts
+++ b/packages/utils/src/get-port.ts
@@ -1,7 +1,4 @@
-import { exec, execFileSync } from 'node:child_process';
-import { promisify } from 'node:util';
-
-const execAsync = promisify(exec);
+import { execFileSync } from 'node:child_process';
/**
* Gets the port number that the process is listening on.
Analysis
Unused import causes TypeScript compilation failure
What fails: TypeScript compiler fails on packages/utils/src/get-port.ts due to unused variable execAsync that is never read in the code.
How to reproduce:
cd packages/utils && pnpm run buildResult:
src/get-port.ts(4,7): error TS6133: 'execAsync' is declared but its value is never read.
Fix: Removed the unused import statements (import { exec, ... and import { promisify ...) and the line const execAsync = promisify(exec); since the code only uses execFileSync instead of the promisified exec function.
| const awkResult = execFileSync( | ||
| 'awk', | ||
| [ | ||
| '/LISTENING/ && /${pid}/ {split($2,a,\":\"); print a[length(a)]; exit}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| '/LISTENING/ && /${pid}/ {split($2,a,\":\"); print a[length(a)]; exit}', | |
| '-v', | |
| `pid=${pid}`, | |
| '/LISTENING/ && $NF == pid {split($2,a,\":\"); print a[length(a)]; exit}', |
The Windows awk pattern / uses invalid awk syntax. The pattern is not valid awk syntax for variable references, which will likely cause the command to fail.
View Details
Analysis
Windows awk pattern fails to extract port due to literal
What fails: The getPort() function on Windows fails to extract the listening port because the awk pattern uses literal instead of passing the PID as an awk variable, causing the pattern to never match and returning undefined.
How to reproduce:
// Call getPort() on Windows when the process is listening on a port
const port = await getPort();
// Returns: undefined (should return the port number like 54321)Actual behavior: The awk command /LISTENING/ && / looks for the literal characters $, {, p, i, d, } in the netstat output instead of matching the actual process ID. This results in no matches and an empty string is returned, which when parsed as an integer yields NaN, returning undefined.
Expected behavior: The awk command should receive the PID as a variable using the -v option so it can properly filter by the process ID and extract the port number. Using awk -v pid=<PID> '/LISTENING/ && $NF == pid' correctly identifies lines with the matching PID.
Testing confirmation: Direct testing of the awk patterns shows:
- Broken pattern:
awk '/LISTENING/ && /returns empty string (no matches) - Fixed pattern:
awk -v pid=1234 '/LISTENING/ && $NF == pid'returns the port number correctly
pid-portfromgetPortmethod because internally it usesnetstatwhich is not reliable