-
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
[css-typed-om] Stub list-valued tests. #10191
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.
Already reviewed downstream.
Build ERROREDStarted: 2018-03-27 09:23:39 Failing Jobs
View more information about this build on: |
Currently we're just adding a TODO when we want to test a list-valued property. It's probably easier if we stub that test function and then implement it later. Bug: 774887 Change-Id: Iab3cea915afbcc8490cacbd45e5632a18e7dece1 Reviewed-on: https://chromium-review.googlesource.com/981956 Reviewed-by: Rune Lillesveen <[email protected]> Commit-Queue: Darren Shen <[email protected]> Cr-Commit-Position: refs/heads/master@{#546067}
c5aa956
to
305c744
Compare
Looks like Firefox timed out because there were too many affected tests: That's because css/css-typed-om/the-stylepropertymap/properties/resources/testsuite.js was changed. It looks like the current test isn't printed, so I can't tell how far in the 10 runs we got. This is #7660 and #10028 and I'm going to bypass Travis to merge anyway. (The change passed Chromium's CQ.) |
Oops! Should I avoid making such large changes in the future? On a related note, is there something I can do in |
@darrnshn, no, please keep breaking our upstream infrastructure until we fix it, don't do any extra work for stuff that works fine in Chromium :) In testsuite.js, if tests fail fast when there API doesn't exist that's quite good enough, and from what I can tell that's the case, I don't see timeouts in Firefox when looking in at a few "random" tests. |
Currently we're just adding a TODO when we want to test a list-valued
property. It's probably easier if we stub that test function and then
implement it later.
Bug: 774887
Change-Id: Iab3cea915afbcc8490cacbd45e5632a18e7dece1
Reviewed-on: https://chromium-review.googlesource.com/981956
Reviewed-by: Rune Lillesveen [email protected]
Commit-Queue: Darren Shen [email protected]
Cr-Commit-Position: refs/heads/master@{#546067}