RE: TestTWF Pull requests needing review (box-sizing-001, box-sizing-002)

Le Ven 8 novembre 2013 1:49, Zhang, Zhiqiang a écrit :
>> From: "Gérard Talbot" [mailto:css21testsuite@gtalbot.org]
>> Sent: Friday, November 8, 2013 4:18 AM
>> To: Zhang, Zhiqiang
>> Cc: Rebecca Hauck; Public CSS testsuite mailing list
>> Subject: RE: TestTWF Pull requests needing review
>>
>>
>> Le Mer 6 novembre 2013 21:46, Zhang, Zhiqiang a écrit :
>> > From: Rebecca Hauck [mailto:rhauck@adobe.com]
>> > Sent: Tuesday, October 29, 2013 6:49 AM
>> >
>> > CSS UI:
>> > -------
>> > https://github.com/w3c/csswg-test/pull/95
>> > https://github.com/w3c/csswg-test/pull/97
>> >
>> > I've reviewed the 2 PRs on GitHub and made an improvement:
>> >
>> > http://zqzhang.github.io/review/box-sizing-001-review.html
>> > http://zqzhang.github.io/review/box-sizing-002-review.html
>> > http://zqzhang.github.io/review/box-sizing-001-ref-review.html
>> >
>> > Any comment is welcome.
>>
>> [Resending this email as my webhost SMTP server seemed to have failed
>> sending it.]
>>
>> Zhiqiang,
>>
>> Here's some feedback on these tests.
>>
>> http://zqzhang.github.io/review/box-sizing-001-review.html
>>
>> line 4:
>> <title>CSS Basic User Interface Test: box-sizing - padding-box</title>
>>
>> Suggestion:
>> <title>CSS Basic User Interface Test: box-sizing - padding-box
>> (basic)</title>
>
> Better, updated.
>
>>
>> or
>>
>> <title>CSS Basic User Interface Test: box-sizing - padding-box
>> (simple)</title>
>>
>>
>> line 6: <link rel="help"
>> href="http://www.w3.org/TR/css3-ui/#box-sizing">
>>
>> Suggestion:
>> <link rel="help" href="http://www.w3.org/TR/css3-ui/#box-sizing"
>> title="6.1. 'box-sizing' property">
>
> Added. Though title is not suggested by reftest template.
>
> http://testthewebforward.org/docs/test-templates.html#reftest-test-case-template-
>


Adding the title helps a bit when there is more than 1 <link rel="help">
to spec and if the user/reviewer uses a document (relations) links bar
in which case the document links bar will display the title text.

See:

link-bar-example.html

shows how I can use such title attribute for a test like

http://test.csswg.org/suites/css2.1/nightly-unstable/html4/margin-em-inherit-001.htm

that has 3 <link rel="help">



>>
>>
>> line 11 to 17:
>>         div {
>>             height: 200px;
>>             left: 10px;
>>             position: absolute;
>>             top: 50px;
>>             width: 200px;
>>         }
>>
>> setting left and top to an arbitrary value is not needed by the test
>> and
>> not necessary in the test.
>
> Yes, agree. I thought to keep the original test code.
>
>>
>> We already have a reftest which is very frequently used and reused for
>> tests which would fit perfectly here:
>>
>> http://test.csswg.org/source/approved/css2.1/src/reference/ref-filled-
>> green-100px-square.xht


This ref-filled-green-100px-square.xht reftest eventually has to be
copied (hg copy) from
/source/approved/css2.1/src/reference/
into where the test will be so that Shepherd is aware that it has to
reuse that same reftest.
I'm just mentioning this here as there is no box-sizing-001 test yet in

http://test.csswg.org/shepherd/search/testcase/spec/css-ui-3/

and there is no /css3-ui/ folder yet in

http://test.csswg.org/source/approved/


>>
>> Shepherd indicates that such reftest is referenced by 151 tests
>> already
>>
>> http://test.csswg.org/shepherd/reference/ref-filled-green-100px-square/
>>
>> So, I would change 200px (height and width) for 100px and then just
>> reuse that reftest. In the long term, reducing the number of reftests
>> brings many benefits: reusability, reduce intricability (N tests to 1
>> reftests instead of multiple 1 test to 1 reftest), more efficient
>> memory
>> management.
>>
>>
>> line 21:
>>             padding: 20px;
>>
>> In other to maximize noticeability of test failure, I would set
>> padding
>> to 50px.
>
> This is very good, updated.
>
>>
>>
>>
>> line 30-31:
>>     <div class="test-overlapped-red"></div>
>>     <div class="ref-overlapping-green"></div>
>>
>> Semantically speaking, class is for logical grouping of several
>> elements. Id is for linking to a document-unique element. So, here, I
>> would use id instead of class.
>
> Agree.
>
> Gérard, since you and I have reviewed the tests, I added you and me as
> reviewers to them. Thank you.

Great! Marvelous! :)

Gérard

>
> hayashih and kasshisatari, feel free to upload the tests.
>
> Thanks,
> Zhiqiang



-- 
Contributions to the CSS 2.1 test suite:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/

CSS 2.1 Test suite RC6, March 23rd 2011:
http://test.csswg.org/suites/css2.1/20110323/html4/toc.html

CSS 2.1 test suite harness:
http://test.csswg.org/harness/

Contributing to to CSS 2.1 test suite:
http://www.gtalbot.org/BrowserBugsSection/css21testsuite/web-authors-contributions-css21-testsuite.html

Received on Friday, 8 November 2013 18:34:39 UTC