Re: 54 new tests submitted in section 3.1

On 12/27/2014 11:45 AM, Gérard Talbot wrote:
> Le 2014-12-25 10:27, 塩澤 元 a écrit :
>> Hi Gérard,
>>
>> I have reviewed the following test.
>>  - block-flow-direction-001.xht ~ block-flow-direction-022.xht
>>  - line-box-direction-001.xht ~ line-box-direction-020.xht
>>
>> Here is comments for these test.
>>
>> 1.
>> Some test-cases specify another specification's ref-file
>> (/css-multicol-1/multicol-count-002-ref.xht) as its ref-file.
>> I think that it is better to make a new ref-file (or copy) in
>> css-writing-modes-3 directory.
>
> Hajime,
>
> We discussed this in
> http://lists.w3.org/Archives/Public/public-css-testsuite/2014Nov/0039.html
>
> I think we eventually should filename-rename
> multicol-count-002-ref.xht
> as
> ref-yellow-PASS-on-black.xht
> or some filename like that and then move that file into
> http://test.csswg.org/source/css21/reference/
> with other frequently reused reference files.

I agree with Hajime on this one, I don't think it's good to reference
a reference file inside a different test suite.  But I also agree with
Gérard, it would be good to have this as a common test reference. In
that case, however, I think it would go in a top-level reference folder,
like the common support files are in a top-level support folder.

>> 2.
>> All test-case uses 'yellow' and 'black' color.
>> I think that it is better to use 'green' and 'blue' because these makes
>> positive impression.
>
> Ideally, you want to use green color and restrict using green color if and only if red indicates failure. Green color should
> be restricted in tests where failures will be indicated by red color.

This is true, and I think Hixie tended to use the color-combination of
blue and yellow for such cases. Black is, I agree, a bit too intense
here. :) We also tend to use black for descriptive text, so it's good
to have a different color than black for the actual test rendering.

>> 3.
>> I think that it is better to add information to title in some test-case.
>>
>> - block-flow-direction-005.xht, block-flow-direction-006.xht
>> before: CSS Writing Modes Test: float and 'vertical-rl' - block flow
>> direction of block-level boxes
>> after: CSS Writing Modes Test: float*-left* and 'vertical-rl' - block flow
>> direction of block-level boxes
>>
>> - block-flow-direction-007.xht, block-flow-direction-007.xht
>> before: CSS Writing Modes Test: float and 'vertical-lr' - block flow
>> direction of block-level boxes
>> after: CSS Writing Modes Test: float*-right* and 'vertical-lr' - block flow
>> direction of block-level boxes
>>
>> - line-box-direction-005.xht, line-box-direction-006.xht
>> before: CSS Writing Modes Test: float and 'vertical-rl' - ordering
>> direction of line boxes
>> after: CSS Writing Modes Test: float*-left* and 'vertical-rl' - ordering
>> direction of line boxes
>>
>> - line-box-direction-007.xht, line-box-direction-008.xht
>> before: CSS Writing Modes Test: float and 'vertical-lr' - ordering
>> direction of line boxes
>> after: CSS Writing Modes Test: float*-right* and 'vertical-lr' - ordering
>> direction of line boxes
>
> I am not sure why specifying the float value is or would be important in the title. The float value is specified in the assert
> text and in the code. Normally, we want title text to be as short as possible while at the same time to be descriptive.
>
> "
> The title is descriptive but not too wordy.
> "
> http://testthewebforward.org/docs/review-checklist.html#reftests-only
>
> "
> The title appears in the generated index, so make sure it is concise, unique and descriptive. The role of the title is to
> identify what specific detail of a feature or combination of features is being tested, so that someone looking through an
> index can see quickly what's tested in which file. In most cases, this description should not require more than 5 or 6 words.
> There is no need to provide the chapter or section in the title.
> "
> https://wiki.csswg.org/test/format#title-element
>
>
> By the way, I use 'float: left' with 'vertical-rl' and 'float: right' with 'vertical-lr'; in order to make a test suite
> coverage more complete and thorough, we probably should test both float values for each vertical writing-mode values ...
>
> Anyway, I will add the float value in the title, as you suggested.

The title should be as concise as possible, but it also needs to be
sufficiently descriptive to uniquely identify the test. So, as short
as possible -- but no shorter!

Also, title should just say "CSS Test:", not "CSS Writing Modes Test:".
We decided not to include the module name to keep it shorter.

~fantasai

Received on Tuesday, 30 December 2014 03:30:26 UTC