Split the big test() up into multiple small ones #27745
Conversation
wpt-pr-bot
requested review from
annevk,
domenic,
foolip,
jdm,
jgraham and
zqzhang
cbiesinger
force-pushed
the
source
branch
2 times, most recently
from
a944d88
to
33ebbc4
Compare
There was a problem hiding this comment.
Could you add getComputedStyle().width/height tests for both the img and the source elements, in all cases? I'm not even sure what the result should be, per https://github.com/whatwg/html/pull/5894/files#r582084336 , but probably you are...
|
Nothing should get mapped to BTW, I am planning to land the blink implementation patch tonight CET together with the test to ensure that it makes it into the branch, but I will certainly make sure to address any further comments afterwards. |
|
@domenic I added tests for that now. |
There was a problem hiding this comment.
Tests LGTM. I would ideally like another set of eyes, such as @emilio or @annevk or @yoavweiss.
|
Thank you! I am submitting them through the Chrome CQ at https://crrev.com/c/2642939 to make merging simpler. I will continue to address any comments even after landing, of course. |
There was a problem hiding this comment.
I have two concerns:
- Many seemingly unrelated tests are grouped together in a single test. It seems that each time
createPicture()is used a new test wrapper could also be used to keep the assertions somewhat isolated. - The setup doesn't wait for the images to be loaded, so all the style tests are about img elements without images, as far as I can tell.
Thanks, good point, I will fix this.
Yes. I don't think this should matter for computed style, and the ones where I also assert_ratio will not work once the image is loaded because once an image is loaded its actual dimensions will override the aspect-ratio. However, I can add some computed style checks with a loaded image. |
Also test the computed style on a <picture> with a loaded image. Addresses the review comments in web-platform-tests#27745
|
@annevk -- I've made the changes in this PR, please take a look |
cbiesinger
changed the title
…all ones, a=testonly Automatic update from web-platform-tests Split the big test() up into multiple small ones Also test the computed style on a <picture> with a loaded image. Addresses the review comments in web-platform-tests/wpt#27745 -- wpt-commits: 945659ffe92b58c711d68e9c15aa047751dfb89e wpt-pr: 27745
…all ones, a=testonly Automatic update from web-platform-tests Split the big test() up into multiple small ones Also test the computed style on a <picture> with a loaded image. Addresses the review comments in web-platform-tests/wpt#27745 -- wpt-commits: 945659ffe92b58c711d68e9c15aa047751dfb89e wpt-pr: 27745 UltraBlame original commit: 19dce124c96becbadde1e2318d3ceae1ad435e1d
…all ones, a=testonly Automatic update from web-platform-tests Split the big test() up into multiple small ones Also test the computed style on a <picture> with a loaded image. Addresses the review comments in web-platform-tests/wpt#27745 -- wpt-commits: 945659ffe92b58c711d68e9c15aa047751dfb89e wpt-pr: 27745 UltraBlame original commit: 19dce124c96becbadde1e2318d3ceae1ad435e1d
…all ones, a=testonly Automatic update from web-platform-tests Split the big test() up into multiple small ones Also test the computed style on a <picture> with a loaded image. Addresses the review comments in web-platform-tests/wpt#27745 -- wpt-commits: 945659ffe92b58c711d68e9c15aa047751dfb89e wpt-pr: 27745 UltraBlame original commit: 19dce124c96becbadde1e2318d3ceae1ad435e1d
Also test the computed style on a with a loaded image.
Addresses the review comments in
#27745