Review comments on css3-background/src/background-clip-007.html

Julien,

http://test.csswg.org/source/approved/css3-background/src/background-clip-007.html

The following comments are in the Shepherd's background-clip-007 file:
http://test.csswg.org/shepherd/testcase/background-clip-007/spec/CSS3-BACKGROUND/#comment-bc5aa48c1aca

I'm relisting them here for various reasons.



Line 9: <meta name="assert" content="background-clip: content-box
properly clips the background color to the content box.">

Ideally, you want the assert text to help reviewers, examiners, people
who are going to look at the source code trying to figure out what this
test is about, its goal, etc. Or when they see the listing of tests in
test suite. Here, I would probably would have said something like:

<meta name="assert" content="When 'background-clip' is set to
'content-box', then the background-color shines only through the content
area; it does not shine through the padding area nor the border area.">

--------

Line 10: <link rel="match" href="reference/background-clip-007-ref.html">

Ideally, you want to reuse an already created and available reftest. We
have a bunch of common and very frequently used/reused reftests
available. It's not only more efficient but also faster, less work and
also involve speedier automated checking of tests.

--------

Lines 12-35: There are 4 CSS rules and 12 CSS declarations. Are all
those necessary, needed in the pursuit of the goal of the test? You want
to use only the relevant CSS code needed for the test. In an ideal test,
you want to shorthen, to reduce to minimum the number of CSS rules and
CSS declarations and only use/declare CSS rules/CSS delcarations that
are really needed for the purpose of the test.

Also, the overlapping technique does not necessarly require relative
positioning *and* absolute positioning. Using on normal flow should
reduce the amount of declarations needed.

The overlapping technique is not a panacea either, is not always the
best choice.

Using class attribute is semantically making sense for logical grouping
of elements; using an id attribute is for accessing unique-in-document
elements.

If the test is about testing 'background-clip: content-box', then I do
not see the need, the purpose of having a blue border for the red
overlapped element.

--------

Line 39: <p>Test passes if there is a green rectangle with a blue border
below and no red visible on the page.</p>

The pass/fail conditions sentence should be as simple, clear,
understandable and as short as possible. Here, it is not.

The test should have at least mentioned a *_square_* and not a
rectangle. The best shape descriptor can often make a difference between
a PASS and a FAIL verdict.

"with a blue border: The reference to the blue border is not relevant to
the test; in fact, the blue border existence should not be part of the
test.

"below": If 90% or so of all tests starts with the pass/fail conditions
sentence, then using the "below" word can be safely removed.

"visible": We assume that testers are not blind persons; this test suite
is for paged and screen test suite. So, "visible" can be safely removed
and such removal would not make the pass/fail conditions sentence less
clear. "no red visible" is not any more sensible than saying the
opposite: "Test passes if there is invisible red in the page."

"on the page": Such bit of info is not meaningful, helpful or necessary.
Removing "on the page" from the pass/fail conditions sentence does not
affect its understanding.

Here's a test that basically has all the above characteristics while
also testing 'background clip: content-box':

http://www.gtalbot.org/BrowserBugsSection/CSS3Backgrounds/backgroundground-clip-234.html

I have added a few other variations of such test in my /CSS3Backgrounds
folder. "234" in the filename is just a random number.

Gérard
-- 
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 Tuesday, 30 October 2012 19:52:51 UTC