Skip to content
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

Websocket wpt tests hangs in CI #1928

Closed
mcollina opened this issue Feb 13, 2023 · 5 comments
Closed

Websocket wpt tests hangs in CI #1928

mcollina opened this issue Feb 13, 2023 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@mcollina
Copy link
Member

As titled, they often hang and they need to be manually terminated.

This does not happen locally.

@mcollina mcollina added the bug Something isn't working label Feb 13, 2023
@mcollina
Copy link
Member Author

cc @KhafraDev

@KhafraDev
Copy link
Member

KhafraDev commented Feb 13, 2023

I think we should disable the websocket WPTs.

What's (probably) happening:

  • websocket WPTs are about to run
  • websocket server is created in a child process
  • tests are loaded and run as expected
  • undici can't connect to the server (I can repro this when using wsl & a vpn). Eventually we get a timeout error and it's propagated to the websocket handshake.
  • the websocket emits an error event, which the WPTs don't listen for.
  • since no tests pass, the runner is never notified that the tests are complete (because they aren't).
  • we have an infinite hang where the tests can never pass and the worker isn't exiting.

Now that's where I got lost. There is no socket, so theoretically nothing should stop the worker from exiting (even unref'ing the worker doesn't cause it to close).

Even if this was fixed, the websocket tests would still fail.


I spent many, many hours working on this ☹️

@ronag
Copy link
Member

ronag commented Feb 14, 2023

I'm ok with disabling...

@mcollina
Copy link
Member Author

since no tests pass, the runner is never notified that the tests are complete (because they aren't).

Is the runner a piece of our machinery? In that case we should have a timeout.

@KhafraDev
Copy link
Member

I haven't seen a timeout in some time, I think this is sorted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants