Both fast/css-grid-layout/resources/grid-columns-rows-get-set.js and fast/css-grid-layout/resources/grid-columns-rows-get-set-multiple.js (the helpers used by the two tests mentioned in the title) are full of boilerplate. Most of the code can be easily refactored in a couple of helper functions reducing the size of those files significantly. This will also simplify the patch for the bug 127033, as I'll have to change the all the computed values to pixels.
Created attachment 221280 [details] Patch
Created attachment 221379 [details] Patch Also refactored named-grid-line-get-set.html
$ git show --shortstat 9 files changed, 332 insertions(+), 692 deletions(-) I'd say goal achieved :)
Comment on attachment 221379 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221379&action=review Without knowing too much about the subject, I suggested some cosmetic changes. > LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt:39 > +PASS getComputedStyle(element, '').getPropertyValue('-webkit-grid-definition-columns') is "18px" Isn't it possible to use ' instead of " for the output? Because either I am missing something or would not be polluting the diff with some of these changes. > LayoutTests/fast/css-grid-layout/resources/grid-definitions-parsing-utils.js:9 > +function testGridDefinitionsSetJSValues(columnValue, rowValue, computedColumnValue, computedRowValue) I would rework this three functions in a testDefinitionsSetJSValues taking a final parameter useGrid and checking for it to be undefined, considering for example true as default parameter.
Comment on attachment 221379 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221379&action=review Thanks for your comments. >> LayoutTests/fast/css-grid-layout/grid-columns-rows-get-set-expected.txt:39 >> +PASS getComputedStyle(element, '').getPropertyValue('-webkit-grid-definition-columns') is "18px" > > Isn't it possible to use ' instead of " for the output? Because either I am missing something or would not be polluting the diff with some of these changes. That's the direct output of JSON.stringify() (used by shouldBeEqualToString which gives us an additional type checking over the old code). The old code used shouldBe() that does not perform any kind of "stringifycation" >> LayoutTests/fast/css-grid-layout/resources/grid-definitions-parsing-utils.js:9 >> +function testGridDefinitionsSetJSValues(columnValue, rowValue, computedColumnValue, computedRowValue) > > I would rework this three functions in a testDefinitionsSetJSValues taking a final parameter useGrid and checking for it to be undefined, considering for example true as default parameter. The reason why it cannot be done this way, it's because some of the parametters (like the computed values) are already optional.
Committed r162346: <http://trac.webkit.org/changeset/162346>