bzp2010 commented on code in PR #13939: URL: https://github.com/apache/apisix/pull/13939#discussion_r4022330605
########## t/node/websocket-proxy.spec.mts: ########## Review Comment: @shreemaan-abhishek I don't agree that these testing errors should be attributed to Jest itself. If you look at these issues and PRs, it’s not hard to see that they’re all related to Lago tests. There have also been tests related to mcp-bridge in the past. What they have in common is that they all require downloading resources, such as container images (docker compose up) and npm dependencies (pnpx) during test execution (rather than beforehand), which relies on network communication. As is well known, GitHub CI’s network is unstable, and operations like pulling container images often result in errors or timeouts. The `test::nginx` test shell always assumes that tests can complete within an ideal timeframe, almost never considering that these tests may depend on external resources and could fail because of them. And to be honest, our implementation of the custom `--exec` extension isn’t perfect either; it relies on mechanisms like `ngx.log` to report and output errors, which can sometimes cause it to exceed the maximum string length limit. In particular, when timeouts occur due to external resources, Jest may hang and fail to output logs. --- However, I do agree with the view that we should no longer rely on the `test::nginx` shell to launch OpenResty in order to run JavaScript-based tests. I will consider building capabilities for JS testing that are consistent with test::nginx for managing the OpenResty lifecycle, including startup, reload, and restart operations. This way, testing will be entirely confined within the JS runtime. Essentially, this involves migrating similar functionality from a legacy Perl codebase (which is also a scripting language and relies on a runtime) to a JS implementation. At the same time, Jest will be replaced with the faster and more modern vitest. This will begin shortly after this PR is completed, so I think it’s acceptable to tolerate this test running in this less formal manner for the time being. For reference, it does not involve the failure scenarios mentioned above; dependencies are always pulled in advance, and no additional containers need to be downloaded. Therefore, timeouts rarely occur. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
