W3C home > Mailing lists > Public > public-css-testsuite@w3.org > November 2014

Re: [css-grid] display-grid, display-inline-grid and parsing-display-grid-inline: review request

From: Gérard Talbot <css21testsuite@gtalbot.org>
Date: Mon, 10 Nov 2014 14:19:18 -0500
To: Manuel Rego Casasnovas <rego@igalia.com>
Cc: Public CSS Test suite mailing list <public-css-testsuite@w3.org>
Message-ID: <00c46ac65799c2b1b5628c5a3e0c3503@gtalbot.org>
Le 2014-11-10 07:44, Manuel Rego Casasnovas a écrit :
> Hi Gérard,
> thanks for the detail review.
> On 09/11/14 04:03, Gérard Talbot wrote:
>> Le 2014-11-03 11:51, Manuel Rego Casasnovas a écrit :
>>>> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/display-grid.xht
>> Filename should be following the file name format test-topic-###.ext :
> Ok, I thought this was only needed when you have more than one test for
> the same file name.

The documentation could be improved or isn't very detailed on this... or 

Let's say someone (you, me, anyone) comes along afterwards and creates a 
test testing, checking that grid containers are not block containers or 
some other testable statement (there are many testable statements[1] in 
§ 3.1) involving 'display: grid'. Then he/she could and should just 
continue with the number following your last test. Or one day, you could 
decide to do more tests (compound, complex, edge, etc.. kinds of tests) 
with or involving 'display: grid'. Then you could and should continue 
with the sequence.

[1]: 4 testable statements
"floats do not intrude into the grid container"
"grid container’s margins do not collapse with the margins of its 
"Grid containers form a containing block for their contents exactly like 
block containers do"
"The overflow property applies to grid containers"

>> As a test creator, you want to achieve an unique and somewhat
>> descriptive filename. In order to achive this, you may have to prefix
>> your filename with the most significant name of the spec.
> Yeah, I think this makes a lot of sense, so I'll prefix my tests with
> "grid-".
>> If you decide to create your tests in XHTML1, then I suggest to 
>> install
>> HTML Validator (extension available for Firefox only at
>> http://users.skynet.be/mgueury/mozilla/ ) which can run offline and 
>> will
>> report any validation error with your tests.
> Sorry for the mess with the XHTML stuff and thanks for the pointer.
> I've used ".xht" extension as suggested in the documentation, so I'll 
> go
> with XHTML unless HTML is preferred (I don't really mind).
>> Nitpicking
>> "For indentation, spaces are preferred over tabs."
>> http://testthewebforward.org/docs/review-checklist.html
> AFAIK, I've used spaces always, I couludn't find any tab in my patch.

Duh! I was wrong then.

Interestingly, the documentation is contradictory! :)

Use tabs rather than spaces for indentation


For indentation, spaces are preferred over tabs.

>> With GitHub, I can not view the test as rendered in a browser (don't
>> know how; I had to copy and save the code into an HTML editor and then
>> open it in a browser ... not convenient!) and I can not verify things
>> like trailing whitespace (whitespace at the end of lines) or Unicode
>> byte order mark (BOM U+FEFF) (Shepherd is able to spot and report 
>> this,
>> btw) and usage of Unix line endings.
> I guess the syncbot is doing this kind of checks, but I cannot tell for
> sure.
> I'm talking about this kind of comments:
> https://github.com/w3c/csswg-test/pull/626#issuecomment-62379089

This is odd..


stated that your tests submitted 7 days ago passed validation

syncbot commented 7 days ago
Automatic validation checks of commit 93504e0 passed.

but it sure did not pass HTML validation... I'm wondering what kind of 
validation GitHub is exactly doing... it could be very basic edition 
kind of validation...

> There's a way to see the files directly but I think it's an external
> service: https://rawgit.com/
> However it's not working properly for XHTML only for HTML I've just
> reported the issue [1].
> Besides, the testharness.js tests won't work.
>> Addendum
>> ********
>> For a reason I do not understand, locally, I must edit
>> <script type="text/javascript" 
>> src="./resources/testharness.js"></script>
>> <script type="text/javascript"
>> src="./resources/testharnessreport.js"></script>
>> because this linkage
>> <script type="text/javascript" 
>> src="/resources/testharness.js"></script>
>> <script type="text/javascript"
>> src="/resources/testharnessreport.js"></script>
>> will not work.
> Checking other tests they're all using "/resources/testharness.js" so I
> think I should keep it as it is.

Yes, yes. Keep using "/resources/testharness.js" as it is. I think I 
just experienced a particularity (environment path) setting with Linux 
on my system.

> I've updated the pull-request with all these changes (and the rest of
> things you commented). Probably you can comment directly in GitHub

<sigh>I am very much a newbie with GitHub right now. ... <groan>

> instead of replying by mail, but any of the options work for me.
> The pull-request: https://github.com/w3c/csswg-test/pull/626

This will have to wait, unfortunately until I can "walk" (even just 
"baby steps") with GitHub...


> The test files:
> *
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-display-grid-001.xht
> *
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-display-inline-grid-001.xht
> *
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-parsing-display-001.xht
> Please, take a new look.
> Thank you very much,
>   Rego
> [1] https://github.com/rgrove/rawgit/issues/46

Test Format Guidelines

Test Style Guidelines

Test Templates

CSS Naming Guidelines

Test Review Checklist

CSS Metadata
Received on Monday, 10 November 2014 19:19:55 UTC

This archive was generated by hypermail 2.4.0 : Friday, 20 January 2023 19:58:20 UTC