-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Update <link> pseudo selector WPTs #27178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review process for this patch is being conducted in the Chromium project.
899d052
to
e17ff4d
Compare
@@ -9,6 +9,17 @@ function getElementsByIds(ids) { | |||
function testSelectorIdsMatch(selector, ids, testName) { | |||
test(function(){ | |||
var elements = document.querySelectorAll(selector); | |||
//assert_unreached('one: ' + JSON.stringify([...elements], null, 2) + '\n\ntwo: ' + JSON.stringify(getElementsByIds(ids), null, 2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These logs and comments should not be included, as they will fail lint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, they're gone
test_follow_link(str); | ||
}); | ||
|
||
async_test(function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing this test fail in Firefox when I try it locally; are you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html shows it still passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was failing in firefox for me, but it was passing in safari which it shouldnt be, so I just changed it a bit - how about now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; failing in Firefox now! https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html
e17ff4d
to
899b59f
Compare
899b59f
to
5bbd383
Compare
test_follow_link(str); | ||
}); | ||
|
||
async_test(function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good; failing in Firefox now! https://wptpr.live/27178/html/infrastructure/urls/resolving-urls/query-encoding/navigation.sub.html
ugh it looks like its flaky on firefox, which kinda makes sense since its a bit racy - I can't think of any better ideas of how to wait for the navigation to occur or not and pass when it doesn't occur... |
I think I figured it out, new patch on the way. |
5bbd383
to
94e368f
Compare
94e368f
to
085de67
Compare
In these spec PRs [1][2], the <link> element will no longer match :link, :visited, or :any-link, and it will no longer navigate the page when clicked. This patch also modifies the TestExpectations for one of the modified tests navigation.sub.html. There was one test case which was consistently timing out which I removed, which would stop the test from timing out. However, there are an additional two test cases which time out when run with run_web_tests.py but pass just fine with run_wpt_tests.py. In addition, the flags passed to run_web_tests.py on the trybots make it so that those two test cases which time out get written to a failure file instead of making the whole test runner time out, which would count as a failure in TestExpectations. Since the default flags for run_web_tests.py make it an actual timeout according to TestExpectations, I left the timeout expectation in there as well. Hopefully this test can just be run with run_wpt_tests.py in the future and we can get rid of this TestExpectation. [1] whatwg/html#6269 [2] w3c/csswg-drafts#5839 Bug: 611093 Change-Id: I7234eb6024450e28c1ca6ec1ede7fdfc8f2ece52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628037 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonfreed@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/master@{#844921}
085de67
to
d0e9420
Compare
In these spec PRs [1][2], the <link> element will no longer match :link,
:visited, or :any-link, and it will no longer navigate the page when
clicked.
This patch also modifies the TestExpectations for one of the modified
tests navigation.sub.html. There was one test case which was
consistently timing out which I removed, which would stop the test from
timing out. However, there are an additional two test cases which time
out when run with run_web_tests.py but pass just fine with
run_wpt_tests.py. In addition, the flags passed to run_web_tests.py on
the trybots make it so that those two test cases which time out get
written to a failure file instead of making the whole test runner time
out, which would count as a failure in TestExpectations. Since the
default flags for run_web_tests.py make it an actual timeout according to
TestExpectations, I left the timeout expectation in there as well.
Hopefully this test can just be run with run_wpt_tests.py in the future
and we can get rid of this TestExpectation.
[1] whatwg/html#6269
[2] w3c/csswg-drafts#5839
Bug: 611093
Change-Id: I7234eb6024450e28c1ca6ec1ede7fdfc8f2ece52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628037
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844921}