Review of Flexbox Intrinsic Size Tests #33242
Description
Some commentary from me and @tabatkins on
https://github.com/web-platform-tests/wpt/tree/master/css/css-flexbox/intrinsic-size
Overall Feedback:
The reftests are focused on creating a green box, which isn't always as rigorous a rendering as we could be checking. For example, the size of the flex container is evident, but the size of the flex items are not. We want the test to fail in as many relevant failure conditions as possible! So for these tests, it would be useful to see the bounds of both the flex container and any items in it, to make sure they're all the right sizes.
(Note that we have a lot of reftests that were written before we had automated testing, and had to be manually checked, so there are a lot of tests which go out of their way to be rapidly evaluated by a human. It's still useful to make the tests render understandably, but a) that doesn't mean the tests should compromise their rigor and b) now that we have reftest automation, we don't need to complexify the test to make it simple enough to evaluate in < 3 seconds.)
Also, the asserts in 001-004 are suuuper helpful for reviewing! Thanks for writing them! It would be nice to also have such asserts in the testharness.js tests. :)
row-001:
- Missing closing div
row-002, row-003:
- Looks correct
row-004:
- Test looks correct, but the assert is slightly off: the flex container's min-content size is capped by the item's flex base size when the item can't grow. (If its contents were smaller, then it could still shrink smaller; and there should be a test for that case as well.)
row-005:
- Tests 1, 2, and 3 look correct.
- Test 4 is incorrect - it appears to have missed the "having floored the flex shrink factor at 1" text in 9.9.1 step 1. With that properly taken into account, the expected width should be 400px. (The min-context flex fraction of the first child is -200px; the second's is -400px. We choose -200px as the larger. The first child then doesn't shrink at all from its flex-basis (as the shrink factor is not floored in step 3), while the second shrinks by 200px, ending at 200px wide. 200+200 = 400 total width for the flexbox.)
- Test 5 looks correct (auto min size wins here, making each 100px wide, for 200px flexbox width)
- Test 6 looks correct (honoring 'width' as an "ideal" size, one of them hits 200 and the other grows to 400 to keep the flex fractions correct)
- Test 7 looks correct (one shrinks, one grows, so the positive flex fraction is chosen; first item has 0 growth, second grows to 100 + (2*50)=200, so 400px total flexbox width)
(the contribution/fraction/math comments on test 6 are very helpful; having the same on all 7 would have been great)