Document what tests in the multicolum test suite are about

[frivoal]

Some tests in the multicol test suite have no indication of what it is they are testing, neither in the form of comments, nor of an explicit assertion.

This makes them hard to work with, and would be nice to fix.

There's at least this one:

• https://github.com/w3c/web-platform-tests/blob/5b857bf79ede0c99761aca3d5562e86ba979e49f/css/css-multicol-1/multicol-rule-shorthand-2.xht

[gsnedders]

There's a very slim chance https://github.com/operasoftware/presto-testo/tree/b5d5d519321bb6da4bc825f92c3f4e84db31be6f/css/multicolumn might be helpful, given they're ultimately derived from those tests (and specifically those in the reftest subdirectory).

[TalbotG]

Some tests in the multicol test suite have no indication of what it is they are testing, neither in the form of comments, nor of an explicit assertion.

Some tests in the multicol test suite give some indication (hint) of what they are testing in their filenames. Normally, unless the test is really basic, all the multicolumn tests that I reviewed and approved have useful comments or explicit assertion.

There's at least this one:

https://github.com/w3c/web-platform-tests/blob/5b857bf79ede0c99761aca3d5562e86ba979e49f/css/css-multicol-1/multicol-rule-shorthand-2.xht

I did not create that multicol-rule-shorthand-2 test.
I did not review that multicol-rule-shorthand-2 test but I can explain it.

The important lines in the test are:
line 28 column-rule: solid blue 1em;
line 29 column-rule: normal red 1em;
line 30 column: normal red 1em;

where lines 29 and 30 are invalid: 'normal' is not a border-style. 'column' is not a valid property name to begin with; 'normal' should only be used for 'column-gap'. So, there should be no red. There should be black (glyphs), gray (border), some yellow (background) and 3 short vertical stripes of blue (separators of column boxes)). So, on top of a few tweakings, I would add the invalid flag and add:

line 29 column-rule: normal red 1em; /* invalid; 'normal' is not a 'border-style' */
line 30 column: normal red 1em; /* invalid: 'column' is not a valid property name; 'normal' can only apply to 'column-gap' */

and I would remove lines 32 to 45 since they are pointless and useless in the test. Giving something like:

http://www.gtalbot.org/BrowserBugsSection/CSS3Multi-Columns/Opera/multicol-rule-shorthand-2-GT.xht

and even there, the test could be furthermore reduced, trimmed in order to focus on what really is being tested: invalid column-rule shorthand. One related problem to further reducing tests (that you did not author) is that you do not want to redo all the tests from Opera since you want the original author to eventually recognize his/her own tests too.

[rachelandrew]

I have a spreadsheet that identifies the ones that need a description and/or assertion so aim to fix this. Thanks for the info re that specific test.

[TalbotG]

the test could be furthermore reduced

line 23: orphans: 1;
line 24: widows: 1;

should be removed from that specific test. Because it is not needed, not necessary and not relevant in that specific test.

There are other tests (eg /CSS3Multi-Columns/Opera/multicol-columns-005-GT.xht is one) where both orphans: 1; widows: 1; can (and should) also be removed. Only a minority of tests in the current test suite could require widows: 1 in order to be fully reliable and trustworthy.
In 2013, I was not familiar with how 'orphans' and 'widows' worked within column boxes and the way these were implemented for column boxes.

[rachelandrew]

Working through tidying these up at the moment, also checking that they relate to current spec text etc. as I'll be linking them in using the new bikeshed functionality. Came across: https://github.com/web-platform-tests/wpt/blob/master/css/css-multicol/multicol-width-003.xht which has no description or assert meta etc.

I can see what it's doing, but I'm wondering what the motivation for this test was (and the spec doesn't really discuss the interaction with abspos).

[TalbotG]

Rachel, my guess is that this
http://test.csswg.org/suites/css-multicol-1_dev/nightly-unstable/html4/multicol-width-003.htm
test is testing whether column box or multicolumn element acts as containing block for absolute positioned elements. In particular, it seems to me to be putting to the test, I suppose, this statement from section 2 of the spec:
"
Column boxes do not become the containing block for elements with position: fixed or position: absolute. The containing block is the multicol element, it being the principal box.
"

I can see what it's doing, but I'm wondering what the motivation for this test was (and the spec doesn't really discuss the interaction with abspos).

The spec mentions interaction with abspos. Look for Example 9

In the 2011 version of the spec., the spec only mentioned this:

"
column boxes do not establish containing blocks for elements with ‘position: fixed’ or ‘position: absolute’.
"
https://www.w3.org/TR/2011/CR-css3-multicol-20110412/

[rachelandrew]

Yes - I made that example image - but it doesn't seem related to column width (which is the only hint this test has as to what it is is, the filename).

[rachelandrew]

I think I'll update it as a test for the containing block as I agree that seems to be what is being tested. I assume it's not a good idea to rename files?

[TalbotG]

I think that
http://test.csswg.org/suites/css-multicol-1_dev/nightly-unstable/html4/multicol-width-003.htm
test could be improved by using right offset instead of left offset. That way, you would be checking in a more reliable and trustworthy manner that a column box can not be acting as a containing block for abs.pos. elements.
Also, in that test, I would remove orphans: 1 and widows: 1

---

I assume it's not a good idea to rename files?

Well, if you create a new test, then it's fine to give it another filename...

[rachelandrew]

I've been going through these. The majority of the legacy tests are now added to the multicol spec using the Bikeshed wpt functionality. While doing that I had to figure out what all the tests were, so it made sense to add the assert lines as I went, which are in PR #13616

If I've misrepresented anything let me know.