-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Submitting ported Microsoft custom props test suite #5288
Conversation
Notifying @SimonSapin, @dbaron, @svgeesus, and @tabatkins. (Learn how reviewing works.) |
Originally posted as w3c/csswg-test#1249 (comment) by @syncbot on 23 Mar 2017, 22:10 UTC:
|
@gsnedders I apologize for my ignorance, but what exactly is Travis saying is failing here? All the meta is setup as expected, is it because FF/Chrome are failing tests? |
@gregwhitworth the fact some are failing is irrelevant: it's the fact that for some of the subtests it's getting 20 results from running the tests 10 times, which is likely caused by default test names (looking quickly in @jgraham @jugglinmike can we make this clearer somehow? |
{ varName:"--var", expectedValue:"value", style:"--var:value;--var:;", testName:"can't overwrite with no value"}, | ||
{ varName:"--var", expectedValue:" ", style:"--var:value;--var: ;", testName:"can overwrite with space value"}, | ||
{ varName:"--var", expectedValue:"value1", style:"--var:value1; --Var:value2", testName:"case sensetivity"}, | ||
{ varName:"--Var", expectedValue:"value2", style:"--var:value1; --Var:value2", testName:"case sensetivity"}, |
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.
Some test name as the line above, also that's not how you spell sensitivity. See the rest of the unstable results found, they're likely all similar.
Sure thing. Check out gh-5305. It's a little subtle since we're working within |
@gsnedders Since you now "own" this merge from what I believe - how do I update my PR? Should I just create a new one? |
@gregwhitworth you now have push access |
Firefox (nightly)Testing web-platform-tests at revision f3d796e All results42 tests ran/css/css-variables-1/vars-font-shorthand-001.html
/css/css-variables-1/variable-animation-from-to.html
/css/css-variables-1/variable-animation-over-transition.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-shorthand.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-transform.html
/css/css-variables-1/variable-animation-substitute-into-keyframe.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-fallback.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-multiple.html
/css/css-variables-1/variable-animation-substitute-within-keyframe.html
/css/css-variables-1/variable-animation-to-only.html
/css/css-variables-1/variable-created-document.html
/css/css-variables-1/variable-created-element.html
/css/css-variables-1/variable-cssText.html
/css/css-variables-1/variable-definition-border-shorthand-serialize.html
/css/css-variables-1/variable-definition-cascading.html
/css/css-variables-1/variable-definition-keywords.html
/css/css-variables-1/variable-definition.html
/css/css-variables-1/variable-first-letter.html
/css/css-variables-1/variable-first-line.html
/css/css-variables-1/variable-invalidation.html
/css/css-variables-1/variable-presentation-attribute.html
/css/css-variables-1/variable-pseudo-element.html
/css/css-variables-1/variable-reference-cssom.html
/css/css-variables-1/variable-reference-refresh.html
/css/css-variables-1/variable-reference-shorthands-cssom.html
/css/css-variables-1/variable-reference-shorthands.html
/css/css-variables-1/variable-reference-variable.html
/css/css-variables-1/variable-reference.html
/css/css-variables-1/variable-substitution-background-properties.html
/css/css-variables-1/variable-substitution-basic.html
/css/css-variables-1/variable-substitution-filters.html
/css/css-variables-1/variable-substitution-plus-box-shadow.html
/css/css-variables-1/variable-substitution-replaced-size.html
/css/css-variables-1/variable-substitution-shadow-properties.html
/css/css-variables-1/variable-substitution-shorthands.html
/css/css-variables-1/variable-substitution-variable-declaration.html
/css/css-variables-1/variable-transitions-from-no-value.html
/css/css-variables-1/variable-transitions-to-no-value.html
/css/css-variables-1/variable-transitions-transition-property-all-before-value.html
/css/css-variables-1/variable-transitions-transition-property-variable-before-value.html
/css/css-variables-1/variable-transitions-value-before-transition-property-all.html
/css/css-variables-1/variable-transitions-value-before-transition-property-variable.html
|
*This report has been truncated because the total content is 68382 characters in length, which is in excess of GitHub.com's limit for comments (65536 characters). Sauce (safari)Testing web-platform-tests at revision f3d796e All results42 tests ran/css/css-variables-1/variable-animation-from-to.html
/css/css-variables-1/variable-animation-over-transition.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-shorthand.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-transform.html
/css/css-variables-1/variable-animation-substitute-into-keyframe.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-fallback.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-multiple.html
/css/css-variables-1/variable-animation-substitute-within-keyframe.html
/css/css-variables-1/variable-animation-to-only.html
/css/css-variables-1/variable-created-document.html
/css/css-variables-1/variable-created-element.html
/css/css-variables-1/variable-cssText.html
/css/css-variables-1/variable-definition-border-shorthand-serialize.html
/css/css-variables-1/variable-definition-cascading.html
/css/css-variables-1/variable-definition-keywords.html
/css/css-variables-1/variable-definition.html
/css/css-variables-1/variable-first-letter.html
/css/css-variables-1/variable-first-line.html
/css/css-variables-1/variable-invalidation.html
/css/css-variables-1/variable-presentation-attribute.html
/css/css-variables-1/variable-pseudo-element.html
/css/css-variables-1/variable-reference-cssom.html
/css/css-variables-1/variable-reference-refresh.html
/css/css-variables-1/variable-reference-shorthands-cssom.html
/css/css-variables-1/variable-reference-shorthands.html
/css/css-variables-1/variable-reference-variable.html
/css/css-variables-1/variable-reference.html
/css/css-variables-1/variable-substitution-background-properties.html
/css/css-variables-1/variable-substitution-basic.html
/css/css-variables-1/variable-substitution-filters.html
/css/css-variables-1/variable-substitution-plus-box-shadow.html
/css/css-variables-1/variable-substitution-replaced-size.html
/css/css-variables-1/variable-substitution-shadow-properties.html
/css/css-variables-1/variable-substitution-shorthands.html
/css/css-variables-1/variable-substitution-variable-declaration.html
/css/css-variables-1/variable-transitions-from-no-value.html
/css/css-variables-1/variable-transitions-to-no-value.html
/css/css-variables-1/variable-transitions-transition-property-all-before-value.html
|
Sauce (MicrosoftEdge)Testing web-platform-tests at revision 3178d1e |
Chrome (unstable)Testing web-platform-tests at revision f3d796e All results42 tests ran/css/css-variables-1/variable-animation-from-to.html
/css/css-variables-1/variable-animation-over-transition.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-shorthand.html
/css/css-variables-1/variable-animation-substitute-into-keyframe-transform.html
/css/css-variables-1/variable-animation-substitute-into-keyframe.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-fallback.html
/css/css-variables-1/variable-animation-substitute-within-keyframe-multiple.html
/css/css-variables-1/variable-animation-substitute-within-keyframe.html
/css/css-variables-1/variable-animation-to-only.html
/css/css-variables-1/variable-created-document.html
/css/css-variables-1/variable-created-element.html
/css/css-variables-1/variable-cssText.html
/css/css-variables-1/variable-definition-border-shorthand-serialize.html
/css/css-variables-1/variable-definition-cascading.html
/css/css-variables-1/variable-definition-keywords.html
/css/css-variables-1/variable-definition.html
/css/css-variables-1/variable-first-letter.html
/css/css-variables-1/variable-first-line.html
/css/css-variables-1/variable-invalidation.html
/css/css-variables-1/variable-presentation-attribute.html
/css/css-variables-1/variable-pseudo-element.html
/css/css-variables-1/variable-reference-cssom.html
/css/css-variables-1/variable-reference-refresh.html
/css/css-variables-1/variable-reference-shorthands-cssom.html
/css/css-variables-1/variable-reference-shorthands.html
/css/css-variables-1/variable-reference-variable.html
/css/css-variables-1/variable-reference.html
/css/css-variables-1/variable-substitution-background-properties.html
/css/css-variables-1/variable-substitution-basic.html
/css/css-variables-1/variable-substitution-filters.html
/css/css-variables-1/variable-substitution-plus-box-shadow.html
/css/css-variables-1/variable-substitution-replaced-size.html
/css/css-variables-1/variable-substitution-shadow-properties.html
/css/css-variables-1/variable-substitution-shorthands.html
/css/css-variables-1/variable-substitution-variable-declaration.html
/css/css-variables-1/variable-transitions-from-no-value.html
/css/css-variables-1/variable-transitions-to-no-value.html
/css/css-variables-1/variable-transitions-transition-property-all-before-value.html
/css/css-variables-1/variable-transitions-transition-property-variable-before-value.html
/css/css-variables-1/variable-transitions-value-before-transition-property-all.html
/css/css-variables-1/variable-transitions-value-before-transition-property-variable.html
/css/css-variables-1/vars-font-shorthand-001.html
|
Codecov Report
@@ Coverage Diff @@
## master #5288 +/- ##
==========================================
+ Coverage 87.22% 87.22% +<.01%
==========================================
Files 24 24
Lines 2418 2419 +1
Branches 406 406
==========================================
+ Hits 2109 2110 +1
- Misses 239 240 +1
+ Partials 70 69 -1
Continue to review full report at Codecov.
|
Ok, @tabatkins @svgeesus @SimonSapin or @dbaron can one of you please review this as it's good to go. |
@gsnedders can we assume the non-terminating Edge bot results are unrelated to this PR? |
@svgeesus yes |
On first look these tests seem good but it will take me a while to check each one. If another reviewer completes a review before I do, please merge |
@svgeesus Any update here? |
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 slightly sucks that interpolated colors are reported differently in different browsers, but that isn't the spec under test here so is fine.
For variable-invalidation.html and related tests I agree this should be tested; they are more integration tests than unit tests, but justified.
In a few cases there is text in comments which could have been put in link rel="assertion" but this is optional and I'm certainly not going to hold up review for that; if it happens great but no biggie.
On vars-font-shorthand-001.html I wasn't sure if the ahem flag should be used, as it doesn't matter if ahem is installed locally; it is provided as a webfont. But again, not holding up approval for that.
These tests are clear and straightforward.
Looks good to me.
Merging despite the unrelated CI fail, as agreed with @gsnedders |
Originally posted as w3c/csswg-test#1249 by @gregwhitworth on 23 Mar 2017, 22:01 UTC: