Open Bug 1455501 Opened 6 years ago Updated 2 years ago

[wpt-sync] Sync PR 10544 - [wptrunner] Re-enable secure WebSocket server

Categories

(Testing :: web-platform-tests, enhancement, P4)

enhancement

Tracking

(Not tracked)

People

(Reporter: mozilla.org, Unassigned)

References

()

Details

(Whiteboard: [wptsync downstream error])

Sync web-platform-tests PR 10544 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/w3c/web-platform-tests/pull/10544
Details from upstream follow.

Mike Pennisi <mike@mikepennisi.com> wrote:
>  [wptrunner] Re-enable secure WebSocket server
>  
>  Commit bfef1f20a419d24633e48d24c14e6a7503e1d48c organized WPT
>  configuration management into a dedicated class. As part of this change,
>  it modified the way certain properties were initialized. The original
>  implementation included the following code:
>  
>      config = serve.merge_json(default_config, local_config)
>  
>  The `merge_json` method operated on object values recursively. Any
>  properties in `default_config["ports"]` that were *not* present in
>  `local_config["ports"]` were persisted.
>  
>  The commit referenced above translated that code to:
>  
>      config = serve.Config(override_ssl_env=self.ssl_env, **default_config)
>      config.ports = {
>          "http": [8000, 8001],
>          "https": [8443],
>          "ws": [8888]
>      }
>  
>  In this factoring, the `config.ports` property is completely
>  overwritten.  Because the `default_config` object contains a "wss"
>  property that is not present in the override, this new approach disables
>  the default secure WebSocket server.
>  
>  Re-implement the original behavior by selectively overwriting only the
>  specified configuration properties and persisting any
>  previously-existing properties.
>  
>  ---
>  
>  As an alternative, it might make sense to remove the `config.ports =` statement from `environment.py` and instead declaratively set those values in `config.default.json`. That could be easier to follow (fewer sources of configuration), but I'm not sure where else `config.default.json` is used. The file currently demonstrates the use of the special value `"auto"`. Users are encouraged to copy the file and make local modifications, so the illustrative purpose may be a good reason to leave it as is.
Whiteboard: [wptsync downstream] → [wptsync downstream error]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.