Bug 213028 - [css-grid] Reimport WPT tests removed in r262809
Summary: [css-grid] Reimport WPT tests removed in r262809
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-10 09:43 PDT by Oriol Brufau
Modified: 2020-06-11 05:48 PDT (History)
6 users (show)

See Also:


Attachments
Patch (116.70 KB, patch)
2020-06-10 10:06 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch (114.85 KB, patch)
2020-06-10 15:43 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oriol Brufau 2020-06-10 09:43:24 PDT
r261841 imported some WPT grid tests, but it was reverted in r262809.
Then these tests have been lost. Should be imported again.
Comment 1 Oriol Brufau 2020-06-10 10:06:13 PDT
Created attachment 401550 [details]
Patch
Comment 2 Oriol Brufau 2020-06-10 15:43:12 PDT
Created attachment 401594 [details]
Patch
Comment 3 Manuel Rego Casasnovas 2020-06-10 22:40:35 PDT
Comment on attachment 401594 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401594&action=review

r=me, but please review the comment and change the expectations for that test accordingly.

> LayoutTests/TestExpectations:1059
> +webkit.org/b/209461 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-item-dynamic-min-contribution-001.html [ Failure ]

I think the usual way to import testharness tests is to have a -expected.txt file with several FAIL messages, instead of marking them as Failure in the TestExpectations file. As that way we'd realize when there's any single change regarding that test, while the other way around we'll only realize when the whole test starts to PASS.
Comment 4 Oriol Brufau 2020-06-11 04:28:39 PDT
(In reply to Manuel Rego Casasnovas from comment #3)
> I think the usual way to import testharness tests is to have a -expected.txt
> file with several FAIL messages, instead of marking them as Failure in the
> TestExpectations file. As that way we'd realize when there's any single
> change regarding that test, while the other way around we'll only realize
> when the whole test starts to PASS.

The problem is that in a subtest, the height of the element becomes (wrongly) the height of the viewport. And the viewport has different heights in different platforms. So it would be messier.
Comment 5 Oriol Brufau 2020-06-11 05:39:37 PDT
Well, maybe it's not the viewport height, but there is some variance. I was getting

FAIL Minimum size: 300% assert_equals: height expected "300px" but got "5400px"

but in an iOS emulator it was something different than 5400px.
Comment 6 EWS 2020-06-11 05:47:23 PDT
Committed r262901: <https://trac.webkit.org/changeset/262901>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 401594 [details].
Comment 7 Radar WebKit Bug Importer 2020-06-11 05:48:18 PDT
<rdar://problem/64251555>