-
Notifications
You must be signed in to change notification settings - Fork 1
Fix timeout clearTimeout bug and edge case handling in bun/deno shims #15
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
Conversation
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
|
@copilot test longer executions, like 7 second execution timing out after 6 seconds, and 7 second executions completing with 10 second timeout |
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Added test for 7 second execution with 10 second timeout (passes). Also validated 7 second execution with 6 second timeout (times out correctly). Both scenarios work as expected in Deno and Bun. Fixed Bun shim to pass timeout to native test runner. (02eb2d2) |
|
. |
|
@copilot New tests failing in node ok 14 - Three-Level Nesting with Mixed Operationsduration_ms: 20.581975 Subtest: Test with timeout = 0 (no timeout)not ok 15 - Test with timeout = 0 (no timeout)duration_ms: 2.163614 Subtest: Test completes within generous timeoutok 16 - Test completes within generous timeoutduration_ms: 50.214306 Subtest: 7 second execution with 10 second timeoutok 17 - 7 second execution with 10 second timeoutduration_ms: 7001.965407 Error: A resource generated asynchronous activity after the test ended. This activity created the error "RangeError [ERR_OUT_OF_RANGE]: The value of "options.timeout" is out of range. It must be >= 0 && <= 2147483647. Received -100" which triggered an unhandledRejection event, caught by the test runner.1..17 |
Co-authored-by: Hexagon <419737+Hexagon@users.noreply.github.com>
Fixed Node.js shim to filter out invalid timeout values (0 and negative). Node.js requires timeout > 0, so now these edge case tests won't pass invalid values to Node's test runner. (cbce133) Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Investigation Summary
Issues Fixed:
if (timeoutId)toif (timeoutId !== -1)to properly handle when setTimeout returns 0&& options.timeout > 0check to treat 0 and negative values as "no timeout"Test Coverage Added:
Node.js Fix:
Node.js requires timeout to be > 0 and <= 2147483647. Updated
transformOptionsto only pass valid positive timeout values to Node's test runner, preventing "test timed out after 0ms" and "ERR_OUT_OF_RANGE" errors.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.