Remove duplicate copies of subdomains/hostnames #8614

gsnedders · 2017-12-07T16:30:47Z

This change is

jgraham · 2017-12-07T16:45:30Z

tools/wptrunner/wptrunner/browsers/firefox.py

@@ -23,7 +23,6 @@
 from ..executors.executormarionette import (MarionetteTestharnessExecutor,
                                            MarionetteRefTestExecutor,
                                            MarionetteWdspecExecutor)
-from ..environment import hostnames


This is also used later in the file, right?

Apparently so. But it passed CI despite that because we disable the unused variable pyflake still. :(

ghost · 2017-12-07T17:05:13Z

Build PASSED

Started: 2017-12-07 16:31:46
Finished: 2017-12-07 16:43:04

View more information about this build on:

ghost · 2018-01-23T22:38:08Z

Build PASSED

Started: 2018-02-09 14:39:04
Finished: 2018-02-09 14:53:30

View more information about this build on:

gsnedders · 2018-01-29T17:00:29Z

tools/wptserve/wptserve/pipes.py

@@ -392,6 +392,11 @@ def config_replacement(match):
            value = request.headers
        elif field == "GET":
            value = FirstWrapper(request.GET)
+        elif field == "domains":


I really don't like this, but I don't think we can reasonably merge domains and not_domains (given we need to know what's meant to resolve when setting up things like hosts files), and I think we want it like this to resolve #9232.

@jgraham @Ms2ger halp.

I think this is probably fine. Maybe write as:

request.server.config.get('not_domains', request.server.config['domains'])

(if that's a real dict

This seems fine to me too.

jgraham

Could you test this by pushing some representative test changes to this branch and ensuring that you still get meaningful results?

jgraham · 2018-01-31T17:57:36Z

tools/wptrunner/wptrunner/browsers/firefox.py

@@ -173,6 +192,8 @@ def __init__(self, logger, binary, prefs_root, test_type, extra_prefs=None, debu
        self.stylo_threads = stylo_threads
        self.chaos_mode_flags = chaos_mode_flags

+        self.server_locations = write_server_locations_file(self.config)


It doesn't look like you clean up this file during teardown. Using NamedTemporaryFile to create it would perhaps solve that for you.

I was just copying what we did with Servo for the hosts file, FWIW.

tempfile.NamedTemporaryFile doesn't work because there's no guarantee whether you can reopen the file a second time. Per the docs:

Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms (it can be so used on Unix; it cannot on Windows NT or later).

I'd somehow missed the cleanup function before, bleh.

jgraham · 2018-01-31T18:55:16Z

tools/wptrunner/wptrunner/tests/browsers/test_firefox.py

+                                      "foo/prefs",
+                                      "testharness.js",
+                                      config={
+                                          "ports": {"http": [1234]},


This test doesn't seem that useful. It ensures that you write something to some file, but it doesn't really check that the contents are right. It seems more useful to set things up so you can check you get the exact content you expect compared against the file you removed. Obviously this would then have to be changed when new servers are added or removed, but it would have the advantage of actually checking that something functional is produced.

gsnedders · 2018-02-05T17:38:30Z

Very oddly, this doesn't fail locally like it does on Travis. o_O

gsnedders · 2018-02-08T15:43:28Z

Fixes #9232.

Due to recent changes in the internals of the WPT CLI [1], it is no longer necessary to parameterize the `host` value. Remove the property from all browser configuration objects, and remove its reference from the templated JSON file. [1] web-platform-tests#8614

A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Also update the signature of the context manager for the `FontInstaller` "environment extra" to accept this second parameter. [1] web-platform-tests#8614

Due to recent changes in the internals of the WPT CLI [1], it is no longer necessary to parameterize the `host` value. Remove the property from all browser configuration objects, and remove its reference from the templated JSON file. [1] web-platform-tests#8614

A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Require all "environment extra" classes to implement the Python callable protocol, and update the `SauceConnect` and `FontInstaller` class accordingly. [1] web-platform-tests#8614

A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Also update the signature of the context manager for the `FontInstaller` "environment extra" to accept this second parameter. [1] web-platform-tests#8614

A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Also update the signature of the context manager for the `FontInstaller` "environment extra" to accept this second parameter. [1] #8614

…ce, a=testonly Automatic update from web-platform-testsCorrect integration with Sauce Labs service (#9484) A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Also update the signature of the context manager for the `FontInstaller` "environment extra" to accept this second parameter. [1] web-platform-tests/wpt#8614 wpt-commits: e56563d2b1e8bd225bf4c8b4682cbd12beeae1a7 wpt-pr: 9484 wpt-commits: e56563d2b1e8bd225bf4c8b4682cbd12beeae1a7 wpt-pr: 9484

…ce, a=testonly Automatic update from web-platform-testsCorrect integration with Sauce Labs service (#9484) A recently merged patch centralized configuration values which defined the domain names available during test execution [1]. That patch was based on a mistaken assumption regarding the location of that value and the stage of the test execution at which it would be available. (Specifically: the value is not present in the environment configuration object when the "environment extras" are initialized.) This interfered with the initialization of the "environment" for the Sauce Labs service, making it impossible to run the tests there. Defer the retrieval of the environment configuration object to the time that object's context manager is activated. Update the call sites to provide the environment configuration at this moment. Also update the signature of the context manager for the `FontInstaller` "environment extra" to accept this second parameter. [1] web-platform-tests/wpt#8614 wpt-commits: e56563d2b1e8bd225bf4c8b4682cbd12beeae1a7 wpt-pr: 9484 wpt-commits: e56563d2b1e8bd225bf4c8b4682cbd12beeae1a7 wpt-pr: 9484 UltraBlame original commit: b05b78e57595a3f62f44169b76961fd54f7355f8

gsnedders added the do not merge yet label Dec 7, 2017

gsnedders self-assigned this Dec 7, 2017

gsnedders requested a review from jgraham December 7, 2017 16:30

wpt-pr-bot added the infra label Dec 7, 2017

wpt-pr-bot requested a review from bobholt December 7, 2017 16:30

jgraham requested changes Dec 7, 2017

View reviewed changes

gsnedders force-pushed the subdomain-cleanup branch from 291af65 to 80480fa Compare January 16, 2018 18:11

foolip removed the request for review from bobholt January 18, 2018 12:56

gsnedders force-pushed the subdomain-cleanup branch from 80480fa to a23f7e5 Compare January 23, 2018 22:12

gsnedders mentioned this pull request Jan 25, 2018

No cross-registrable-domain host is available for testing. #2669

Closed

gsnedders force-pushed the subdomain-cleanup branch 4 times, most recently from cf9663e to 816e9f3 Compare January 29, 2018 12:30

gsnedders removed the do not merge yet label Jan 29, 2018

gsnedders commented Jan 29, 2018

View reviewed changes

jgraham requested changes Jan 31, 2018

View reviewed changes

wpt-pr-bot added the dom label Feb 1, 2018

wpt-pr-bot requested review from ayg, jdm and zqzhang February 1, 2018 00:30

gsnedders force-pushed the subdomain-cleanup branch 2 times, most recently from e2667ac to eacfb21 Compare February 5, 2018 14:55

gsnedders closed this Feb 5, 2018

gsnedders reopened this Feb 5, 2018

gsnedders closed this Feb 5, 2018

gsnedders reopened this Feb 5, 2018

wpt-pr-bot requested a review from sideshowbarker February 8, 2018 15:37

jgraham approved these changes Feb 9, 2018

View reviewed changes

gsnedders added 4 commits February 9, 2018 14:38

Remove dead code (wptrunner.hosts)

764ceb2

Remove default host from browser env options

24b0932

Stop using server-locations.txt

af680f5

Remove duplicate copies of subdomains/hostnames

e2351c0

gsnedders force-pushed the subdomain-cleanup branch from f279369 to e2351c0 Compare February 9, 2018 14:38

gsnedders merged commit 040654c into web-platform-tests:master Feb 9, 2018

gsnedders deleted the subdomain-cleanup branch February 9, 2018 15:01

jdm mentioned this pull request Feb 12, 2018

Fix import error for Servo executor. #9476

Merged

jugglinmike mentioned this pull request Feb 12, 2018

Fix hostnames #9480

Merged

jugglinmike mentioned this pull request Feb 12, 2018

Correct integration with Sauce Labs service #9484

Merged

This was referenced Feb 15, 2018

Travis CI is failing on Chrome, Safari, and Edge #9520

Closed

{{domains[non-existent]}} doesn't work #9232

Closed

gsnedders mentioned this pull request Feb 15, 2018

Some wpt integration tests being skipped because port in use #9546

Open

foolip mentioned this pull request Feb 21, 2018

Isolated domain for cookie tests? #8581

Closed

foolip mentioned this pull request Mar 9, 2018

Agree on policy on reverting #9953

Closed

gsnedders mentioned this pull request Mar 29, 2018

nonexistent-origin.web-platform.test does not have a corresponding domains entry #8390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove duplicate copies of subdomains/hostnames #8614

Remove duplicate copies of subdomains/hostnames #8614

gsnedders commented Dec 7, 2017 •

edited by wpt-reviewable-bot

Loading

jgraham Dec 7, 2017

gsnedders Dec 7, 2017

ghost commented Dec 7, 2017

ghost commented Jan 23, 2018 •

edited by ghost

Loading

gsnedders Jan 29, 2018

Ms2ger Jan 29, 2018

jgraham Jan 29, 2018

jgraham left a comment

jgraham Jan 31, 2018

gsnedders Jan 31, 2018

jgraham Jan 31, 2018

gsnedders commented Feb 5, 2018

gsnedders commented Feb 8, 2018

Remove duplicate copies of subdomains/hostnames #8614

Remove duplicate copies of subdomains/hostnames #8614

Conversation

gsnedders commented Dec 7, 2017 • edited by wpt-reviewable-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Dec 7, 2017

Build PASSED

ghost commented Jan 23, 2018 • edited by ghost Loading

Build PASSED

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgraham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsnedders commented Feb 5, 2018

gsnedders commented Feb 8, 2018

gsnedders commented Dec 7, 2017 •

edited by wpt-reviewable-bot

Loading

ghost commented Jan 23, 2018 •

edited by ghost

Loading