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

ParentNode-querySelector-All WPT test not working? #6663

Closed
frewsxcv opened this issue Jul 19, 2015 · 7 comments
Closed

ParentNode-querySelector-All WPT test not working? #6663

frewsxcv opened this issue Jul 19, 2015 · 7 comments
Labels

Comments

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jul 19, 2015

Sorry for the brevity of this Issue, I am currently on mobile. From IRC:

03:00   frewsxcv    is anyone familiar with tests/wpt/web-platform-tests/dom/nodes/selectors.js? it appears to be used by tests/wpt/web-platform-tests/dom/nodes/ParentNode-querySelector-All.html but if I try to intentionally make selectors.js fail, ParentNode-querySelector-All.html still passes, so I'm a little confused by that. If anyone has ideas, let me know
03:35   jdm frewsxcv: used how?
03:35   * jdm doesn't see it in http://mxr.mozilla.org/servo/sourc...de_querySelectorAll.html?force=1
03:35   jdm oh, whoops wrong fiel
03:35   frewsxcv    See what?
03:36   jdm frewsxcv: what are you changing that should make it fail?
03:36   frewsxcv    It's selectors.js is included in that html test
03:36   frewsxcv    Just making a valid selector invalid
03:38   jdm hmm
03:38   jdm we don't have an ini file for that test, so in theory it should be entirely passing right now
03:38   jdm so that's strange
03:43   frewsxcv    Yeah, I doubt it entirely passes
03:43   frewsxcv    Because :empty hasn't merged yet
03:50   jdm frewsxcv: hmm, it completes in .5 seconds with 1/1 subtests passed according to http://build.servo.org/builders/m...s/2561/steps/shell_2/logs/stdio
03:50   jdm waffles: go for it!
03:51   frewsxcv    Indicating it is probably not working correctly, right?
03:51   frewsxcv    Maybe we should open an issue?
03:51   jdm frewsxcv: mhm
03:51   jdm yes please
@jdm jdm added the A-testing label Jul 19, 2015
@jdm
Copy link
Member

@jdm jdm commented Jul 19, 2015

Aha, the test loads an iframe and all of the expected tests run off of the iframe load event. However, the main document load event never fires, so the overall test is never considered finished, yet that doesn't get treated as a timeout for some reason.

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Jul 19, 2015

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Jul 19, 2015

@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Jul 19, 2015

web-platform-tests/wpt#2019 merged. Maybe time for a WPT upgrade

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Jul 19, 2015

I can do that, but that won't really help; it'll just make that test be recognized as timing out.

@jdm
Copy link
Member

@jdm jdm commented Jul 19, 2015

The way to fix the timeout here is probably to add a method to document_loader.rs that pushes a new element to blocking_loads without setting up any new request, and call that and finish_load from http://mxr.mozilla.org/servo/source/components/script/dom/htmliframeelement.rs#125 and http://mxr.mozilla.org/servo/source/components/script/dom/document.rs#1891 respectively.

@jdm
Copy link
Member

@jdm jdm commented Jul 20, 2015

Yeah, my mistake: the load event does fire, but it fires before everything else hence why the test "finishes" so quickly.

jdm added a commit to jdm/servo that referenced this issue Jul 20, 2015
jdm added a commit to jdm/servo that referenced this issue Jul 20, 2015
jdm added a commit to jdm/servo that referenced this issue Aug 30, 2015
jdm added a commit to jdm/servo that referenced this issue Sep 3, 2015
jdm added a commit to jdm/servo that referenced this issue Sep 3, 2015
jdm added a commit to jdm/servo that referenced this issue Oct 20, 2015
jdm added a commit to jdm/servo that referenced this issue Oct 20, 2015
jdm added a commit to jdm/servo that referenced this issue Oct 21, 2015
jdm added a commit to jdm/servo that referenced this issue Oct 22, 2015
jdm added a commit to jdm/servo that referenced this issue Oct 26, 2015
jdm added a commit to jdm/servo that referenced this issue Dec 30, 2015
jdm added a commit to jdm/servo that referenced this issue Dec 31, 2015
jdm added a commit to jdm/servo that referenced this issue Dec 31, 2015
jdm added a commit to jdm/servo that referenced this issue Jan 7, 2016
jdm added a commit to jdm/servo that referenced this issue Jan 7, 2016
Ms2ger added a commit to Ms2ger/servo that referenced this issue Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.