Re: [css-grid] grid-vertical-align-001 and grid-inline-vertical-align-001: review request

Le 2014-12-09 10:18, Manuel Rego Casasnovas a écrit :
> Hi Gérard,
> 
> thanks for the detailed review.
> 
> On 08/12/14 03:02, Gérard Talbot wrote:
>> I did not understand why you needed to use empty inline-blocks in your
>> code. Maybe what you coded was okay... I don't know for sure.
> 
> Yeah, you're right I like much more your approach, so I've modified the
> tests, you can take a look to the new versions at:
> 
> * grid-vertical-align-001:
>   * Source:
> https://github.com/mrego/csswg-test/blob/css-grid-vertical-align-tests/css-grid-1/grid-model/grid-vertical-align-001.xht
>   * View:
> https://rawgit.com/mrego/csswg-test/css-grid-vertical-align-tests/css-grid-1/grid-model/grid-vertical-align-001.xht
> 

If you want to perfect your CSS code...

Class is (best used) for logical grouping of elements; id is for 
document-unique elements. So,


             .test-grid-overlapping-green {
                 display: grid;
                 font: 50px/1 Ahem;
                 color: green;
             }

             .vertical-align {
                 vertical-align: 125px;
             }

(...)

         <div class="test-grid-overlapping-green">
             <span>1s</span>
             <span class="vertical-align">2n</span>
         </div>

can be replaced with id in grid-vertical-align-001 and 
grid-inline-vertical-align-001 tests .

> * grid-inline-vertical-align-001:
>   * Source:
> https://github.com/mrego/csswg-test/blob/css-grid-vertical-align-tests/css-grid-1/grid-model/grid-inline-vertical-align-001.xht
>   * View:
> https://rawgit.com/mrego/csswg-test/css-grid-vertical-align-tests/css-grid-1/grid-model/grid-inline-vertical-align-001.xht
> 
>> Ideally, we always want test to fail when a property or property value
>> is not supported or not implemented.

"The test fails when it's supposed to fail"
http://testthewebforward.org/docs/review-checklist.html#all-tests
http://testthewebforward.org/docs/test-style-guidelines.html#key-aspects-of-a-well-designed-test

Generally speaking, you want tests to be stringent, demanding, not 
lenient, not tolerant, not merciful.

An ideal test is one that has a very narrow corridor of success and very 
large corridor of failure.

Test suite coverage:
An ideal test is one that checks, verifies one (at a time) and only one 
single aspect of the spec, like one single testable statement of the 
spec.

> Ups, I didn't realize before. I've modified it following a similar
> approach to previous tests. I understand that it should fail if it's 
> not
> supported, so the new version will fail in that case.
> 
> * grid-display-grid-001.xht:
>   * Source:
> https://github.com/mrego/csswg-test/blob/css-grid-fix-display-grid-test/css-grid-1/grid-model/grid-display-grid-001.xht
>   * View:
> https://rawgit.com/mrego/csswg-test/css-grid-fix-display-grid-test/css-grid-1/grid-model/grid-display-grid-001.xht
> 
> Please tell me if you're ok with the change.
> 
> Cheers,
>   Rego

Later, you will need to get one of the spec editors to check at least a 
minority of your tests because I am not familiar with the grid spec. I 
can review the other aspects (like format, filename, text assert, 
possible pitfalls, etc.) of your tests.

Gérard
-- 
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Received on Tuesday, 9 December 2014 18:53:23 UTC