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

Switch external_host to host_ip. #9651

Merged
merged 2 commits into from Mar 2, 2018

Conversation

Projects
None yet
4 participants
@jgraham
Copy link
Contributor

jgraham commented Feb 23, 2018

When running tests in Firefox we don't require that the hostname is
actually resolvable since we set up some internal DNS overrides. This
means that we don't actually need to have web-platform.test defined in
/etc/hosts. As a consequence when we check the server is responding,
the check will typically fail for Firefox developers if we try to
access by hostname. Previously we made use of a special dance in which
external_host held the ip at the point of the check and was later
replaced by host to actually run tests. But that changed, and instead
we relied on the fact that we (accidentially) didn't actually fail in
the case the servers couldn't be reached by host name. However in some
cases things still failed because local DNS resolved web-patform.test
to some ip address, causing hangs or other badness.

In this patch we simply add a host_ip property to the config, and
connect to that to test the servers are repopnsing rather than the
hostname. We also actually fil if we didn't manage to connect to all
the servers.


This change is Reviewable

@jgraham jgraham requested a review from gsnedders Feb 23, 2018

@wpt-pr-bot wpt-pr-bot added the infra label Feb 23, 2018

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Feb 26, 2018

Do we have any test that catches the previous breakage (i.e., #9650 (comment))? Enough of our tests involve starting the server, so how did it pass CI?

@gsnedders gsnedders closed this Feb 28, 2018

@gsnedders gsnedders reopened this Feb 28, 2018

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 28, 2018

Build PASSED

Started: 2018-03-02 17:18:33
Finished: 2018-03-02 17:34:15

View more information about this build on:

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Mar 1, 2018

It passed CI because if you have the hosts defined in /etc/hosts then nothing was broken. This only makes a difference if you are running Fx and don't have /etc/hosts entries.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Mar 2, 2018

@jgraham can we test this somehow?

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Mar 2, 2018

We could not create /etc/hosts when running the infra tests in firefox. Or I guess we could check the generated config, but that wouldn't ensure that the server uses it in the right way.

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Mar 2, 2018

Waitaminute, this is exactly what #9726 tests. Can you have a look at that?

jgraham added some commits Feb 23, 2018

Switch external_host to host_ip. (#9258)
When running tests in Firefox we don't require that the hostname is
actually resolvable since we set up some internal DNS overrides. This
means that we don't actually need to have web-platform.test defined in
/etc/hosts. As a consequence when we check the server is responding,
the check will typically fail for Firefox developers if we try to
access by hostname. Previously we made use of a special dance in which
external_host held the ip at the point of the check and was later
replaced by host to actually run tests. But that changed, and instead
we relied on the fact that we (accidentially) didn't actually fail in
the case the servers couldn't be reached by host name. However in some
cases things still failed because local DNS resolved web-patform.test
to some ip address, causing hangs or other badness.

In this patch we simply add a host_ip property to the config, and
connect to that to test the servers are repopnsing rather than the
hostname. We also actually fil if we didn't manage to connect to all
the servers.

@jgraham jgraham force-pushed the host_ip branch from aa7c2b8 to 07fdaeb Mar 2, 2018

@gsnedders gsnedders merged commit 26cd868 into master Mar 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gsnedders gsnedders deleted the host_ip branch Mar 2, 2018

@gsnedders gsnedders referenced this pull request Mar 2, 2018

Merged

Test serve start config #9726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.