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

[wptserve] Make .headers work with generated tests #26014

Merged
merged 2 commits into from
Oct 7, 2020

Conversation

Hexcles
Copy link
Member

@Hexcles Hexcles commented Oct 6, 2020

Honour *.headers (including *.sub.headers and dir{.sub}.headers)
when serving tests automatically generated from *.js files. The
documentation doesn't mention this exception and reasonable people would
assume *.headers work the same way as with static files. Add a
functional test, too.

Fixes https://crbug.com/1135559.

Drive-by: some stylistic changes and docstrings.

Honour *.headers (including *.sub.headers and __dir__{.sub}.headers)
when serving tests automatically generated from *.js files. The
documentation doesn't mention this exception and reasonable people would
assume *.headers work the same way as with static files. Add a
functional test, too.

Fixes https://crbug.com/1135559.

Drive-by: some stylistic changes and docstrings.
@wpt-pr-bot wpt-pr-bot temporarily deployed to wpt-preview-26014 October 6, 2020 21:25 Inactive
tools/wptserve/wptserve/handlers.py Outdated Show resolved Hide resolved
tools/wptserve/wptserve/handlers.py Outdated Show resolved Hide resolved
tools/wptserve/wptserve/handlers.py Outdated Show resolved Hide resolved
@@ -73,7 +73,9 @@ def __call__(self, request, response):
self.handler(request, response)

def handle_request(self, request, response):
for header_name, header_value in self.headers:
headers = self.headers + handlers.load_headers(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: I'd consider renaming self.headers to something like self.base_headers. It's just a bit weird to say "these are the headers, but then we have to add more headers on-top to make up the headers".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty much an implementation detail (despite being public), as it's not intended/suitable for direct use, so I'm not very worried about the name here.

Co-authored-by: Stephen McGruer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants