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

[cookies] Add test for default path calculation #14182

Merged
merged 2 commits into from
Nov 22, 2018

Conversation

jugglinmike
Copy link
Contributor

@mikewest This is the cause of many of the failures reported by wpt.fyi for cookies/http-state in Firefox, but it's difficult to understand the problem in those terms.

browser status
Chrome 70 ✔️
Edge 15
Firefox 63
IE 11
Safari 12 ✔️

@jugglinmike
Copy link
Contributor Author

We should probably accommodate the aberrant behavior in the http-state tests since they are not intended to verify default path calculation. I've added a commit to do that.

Here are the results for that directory in Firefox on master today (i.e. ./wpt run --no-restart-on-unexpected firefox cookies/http-state)

web-platform-test
~~~~~~~~~~~~~~~~~
Ran 232 checks (11 tests, 221 subtests)
Expected results: 19
Unexpected results: 213
  subtest: 213 (213 fail)

Restarting after failures alleviates the problem to some degree, though not completely and at the cost of a slower time-to-results (./wpt run firefox cookies/http-state)

web-platform-test
~~~~~~~~~~~~~~~~~
Ran 232 checks (11 tests, 221 subtests)
Expected results: 77
Unexpected results: 155
  subtest: 155 (155 fail)

Firefox fares much better with this patch applied (./wpt run firefox cookies/http-state)

web-platform-test
~~~~~~~~~~~~~~~~~
Ran 232 checks (11 tests, 221 subtests)
Expected results: 169
Unexpected results: 63
  subtest: 63 (63 fail)

...and those improvements persist even when running without restarting after failures (./wpt run --no-restart-on-unexpected firefox cookies/http-state)

web-platform-test
~~~~~~~~~~~~~~~~~
Ran 232 checks (11 tests, 221 subtests)
Expected results: 169
Unexpected results: 63
  subtest: 63 (63 fail)

Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for digging into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants