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

Change the way testharness unit tests start the server #29344

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Jun 11, 2021

This allows them to reuse the same logic as wptrunner to wait for the server to be up, rather than rolling their own.

This is generic logic to check that all the server processes have
started, which seems useful to any consumer trying to start multiple
servers in the background. Therefore it makes more sense for it to
live in the serve module than in wptrunner.
We previously imported the serve module to build the config, but then
actually ran the server using `wpt serve` as a subprocess. This meant
we had to re-implement the logic to check the servers were started and
got it wrong (in particular, we were checking the http server but
using the https server for tests).

I removed the start() method and just had the constructor do the
connection because using the class in an unconnected state doesn't
make sense.
@wpt-pr-bot wpt-pr-bot added infra serve testharness.js wptrunner The automated test runner, commonly called through ./wpt run labels Jun 11, 2021
@wpt-pr-bot wpt-pr-bot requested a review from foolip June 11, 2021 12:50
@jgraham jgraham mentioned this pull request Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra serve testharness.js wptrunner The automated test runner, commonly called through ./wpt run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants