- From: Manuel Rego Casasnovas <rego@igalia.com>
- Date: Mon, 10 Nov 2014 13:44:26 +0100
- To: Gérard Talbot <css21testsuite@gtalbot.org>
- CC: public-css-testsuite@w3.org
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. > 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. > 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 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. I've updated the pull-request with all these changes (and the rest of things you commented). Probably you can comment directly in GitHub instead of replying by mail, but any of the options work for me. The pull-request: https://github.com/w3c/csswg-test/pull/626 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
Received on Monday, 10 November 2014 12:45:04 UTC