Bug 205067 – [css-grid] Remove redundant grid.css files

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

Refer all grid layout tests to a single grid.css file, now located under css/support folder. 
Remove grid.css duplicates inside single test folders, and change path reference in all relative tests.

See PR #20337: https://github.com/web-platform-tests/wpt/pull/20337/files

Created attachment 385280 [details]
Patch

Created attachment 385284 [details]
Patch

[Javier Fernandez]

Comment on attachment 385284 [details]
Patch

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

> LayoutTests/imported/w3c/ChangeLog:78
> +        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-margins-003.html: Added.

Better avoid adding new tests in this patch, as we are only changing the relative paths after removing the redundant resource files.

> LayoutTests/imported/w3c/ChangeLog:4499
> +        (#�):

Be careful with these unexpected changes.  Please, remove the from the patch.

> LayoutTests/imported/w3c/ChangeLog:4507
> +        (#�):

Ditto

> LayoutTests/imported/w3c/ChangeLog:4509
> +        (#�):

Ditto

> LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-template-columns-fit-content-001.html:-75
> -	<div class="item">XXX</div>

We should exclude these changes as well, since maybe they are part of the fix in the code that we are not landed yet.

Created attachment 385593 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 385593 [details]
Patch

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

> LayoutTests/imported/w3c/ChangeLog:4464
> +        (#?:

Please avoid this change.

> LayoutTests/imported/w3c/ChangeLog:4472
> +        (#?:

Ditto.

> LayoutTests/imported/w3c/ChangeLog:4474
> +        (#?:

Ditto.

> LayoutTests/imported/w3c/resources/import-expectations.json:84
> +    "web-platform-tests/css/css-grid/grid-definition": "import", 

This line is not needed.

Actually just one line would be good enough. It's not a problem that more are added but we don't need duplicated ones.
You can either remove this one, or remove all the subfolder lines and just keep the main one like:
"web-platform-tests/css/css-grid": "import",

Created attachment 385752 [details]
Patch

Created attachment 385756 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 385756 [details]
Patch

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

This one looks good, thanks.

> LayoutTests/imported/w3c/ChangeLog:8
> +        Reference all grid tests to a single grid.css inside and remove redundant grid.css files, inside the single test folders.

Nit: Maybe rename this sentence to something like "Reference all grid tests to a single grid.css inside css/support/ and remove redundant grid.css files inside css/css-grid/ subfolders."

[Manuel Rego Casasnovas]

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

This one looks good, thanks.

> LayoutTests/imported/w3c/ChangeLog:8
> +        Reference all grid tests to a single grid.css inside and remove redundant grid.css files, inside the single test folders.

Nit: Maybe rename this sentence to something like "Reference all grid tests to a single grid.css inside css/support/ and remove redundant grid.css files inside css/css-grid/ subfolders."

Created attachment 385877 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 385877 [details]
Patch

r=me, let's wait for EWSs before landing. Thanks.

[Manuel Rego Casasnovas]

Comment on attachment 385877 [details]
Patch

EWSs are green, so let's land this.

[WebKit Commit Bot]

Comment on attachment 385877 [details]
Patch

Clearing flags on attachment: 385877

Committed r253629: <https://trac.webkit.org/changeset/253629>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58005905>