- 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>
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 explanatory... 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 contents" "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 0.9.5.8 (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 " http://testthewebforward.org/docs/test-format-guidelines.html#style-rules versus " For indentation, spaces are preferred over tabs. " http://testthewebforward.org/docs/review-checklist.html >> 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.. https://github.com/w3c/csswg-test/pull/626#issuecomment-61505965 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... Gérard > > 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 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 Monday, 10 November 2014 19:19:55 UTC