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

Add default paths for WPT tests #23174

Merged
merged 2 commits into from Apr 7, 2019

Conversation

Projects
None yet
6 participants
@soniasingla
Copy link
Contributor

commented Apr 7, 2019

Set the following default configs for WPT tests.

--host-key-path=tests\wpt\web-platform-tests\tools\certs\web-platform.test.key
--host-cert-path=tests\wpt\web-platform-tests\tools\certs\web-platform.test.pem
--ssl-type=pregenerated
  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #23133

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented Apr 7, 2019

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @ferjm (or someone else) soon.

@highfive

This comment has been minimized.

Copy link

commented Apr 7, 2019

Heads up! This PR modifies the following files:

@@ -74,6 +78,18 @@ def set_defaults(kwargs):
if kwargs["processes"] is None:
kwargs["processes"] = multiprocessing.cpu_count()

if kwargs["ca_cert_path"] is None:

This comment has been minimized.

Copy link
@CYBAI

CYBAI Apr 7, 2019

Collaborator

Maybe we'll want to check "ca-cert-path" in paths like the following one 👀? (Same for other new added args)

if kwargs["include_manifest"] is None and "include_manifest" in paths:

This comment has been minimized.

Copy link
@jdm

jdm Apr 7, 2019

Member

I see that the existing code has those checks, but I don't see a good reason for them.

@jdm

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

One tidy check failed:

./tests/wpt/run.py:27: E124 closing bracket does not match visual indentation

@jdm jdm added S-fails-tidy and removed S-awaiting-review labels Apr 7, 2019

@jdm jdm assigned jdm and unassigned ferjm Apr 7, 2019

@jdm

This comment has been minimized.

Copy link
Member

commented Apr 7, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2019

📌 Commit 9d48ca1 has been approved by jdm

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2019

⌛️ Testing commit 9d48ca1 with merge 8073e9a...

bors-servo added a commit that referenced this pull request Apr 7, 2019

Auto merge of #23174 - soniasingla:DefaultPaths, r=jdm
Add default paths for WPT tests

Set the following default configs for WPT tests.
```--ca-cert-path=tests\wpt\web-platform-tests\tools\certs\cacert.pem
--host-key-path=tests\wpt\web-platform-tests\tools\certs\web-platform.test.key
--host-cert-path=tests\wpt\web-platform-tests\tools\certs\web-platform.test.pem
--ssl-type=pregenerated
```
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #23133

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23174)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2019

@bors-servo bors-servo merged commit 9d48ca1 into servo:master Apr 7, 2019

4 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
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.