Added parsing tests for properties in CSS Borders 4

[SebastianZ]

This adds an initial set of parsing tests related to the CSS properties introduced in CSS Borders 4.

Sebastian

[wpt-pr-bot]

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

[TalbotG]

This Pull Request is the continuation of Pull Request 42345 and is related to it.

[gsnedders]

This Pull Request is the continuation of Pull Request 42345 and is related to it.

Does this supersede the earlier PR? If so, that should be closed.

[gsnedders]

In general, we don't have versioned directories (c.f. https://github.com/web-platform-tests/wpt/issues/44527); given the draft spec isn't ready for implementation, a file like css/css-borders-4/parsing/border-block-end-radius-computed.html in this PR should probably be renamed to css/css-borders/parsing/border-block-end-radius-computed.tentative.html.

[SebastianZ]

This Pull Request is the continuation of Pull Request 42345 and is related to it.

Does this supersede the earlier PR? If so, that should be closed.

It doesn't. @TalbotG just asked me to split the parsing tests from the rest (point 20), so it's easier to review them.

In general, we don't have versioned directories (c.f. #44527);

I saw that there are basically no versioned directories. Though being able to check which tests are for which level of the spec. is required to see how well that spec. level is covered by tests. So how do you distinguish the tests between the different spec. levels then?

Also note that there is already a css-borders folder with tests that actually target features of CSS Backgrounds 3. So I assume those should be moved to css-backgrounds then, right?

Though I guess that's something to discuss in the other issue.

given the draft spec isn't ready for implementation, a file like css/css-borders-4/parsing/border-block-end-radius-computed.html in this PR should probably be renamed to css/css-borders/parsing/border-block-end-radius-computed.tentative.html.

Disregarding the versioning issue, I can add a "tentative" suffix as suggested as long as the spec. doesn't have a FPWD yet, if that's required.

Sebastian

[TalbotG]

Sebastian,

" .tentative : Indicates that a test makes assertions not yet required by any specification, (...) This flag can be enabled for an entire directory (and all its descendents), by naming the directory ‘tentative’. For example, every test underneath ‘foo/tentative/’ will be considered tentative. " coming from http://web-platform-tests.org/writing-tests/file-names.html#test-features

being able to check which tests are for which level of the spec. is required to see how well that spec. level is covered by tests. So how do you distinguish the tests between the different spec. levels then?

I agree with you. You have a point there. This is a weakness of the WPT implementation.

there is already a css-borders folder with tests that actually target features of CSS Backgrounds 3.

It was decided to split future levels of CSS Backgrounds and Borders into css/css-backgrounds/ and css/css-borders/

[SebastianZ]

there is already a css-borders folder with tests that actually target features of CSS Backgrounds 3.

It was decided to split future levels of CSS Backgrounds and Borders into css/css-backgrounds/ and css/css-borders/

Well, it was my request to split the specs. Though the tests in the css-borders folder were created by some people long before that split and cover features introduced in CSS Backgrounds 3 (and which are not yet added to CSS Borders 4 as it is still a delta specification).

Sebastian

[SebastianZ]

" .tentative : Indicates that a test makes assertions not yet required by any specification, (...) This flag can be enabled for an entire directory (and all its descendents), by naming the directory ‘tentative’. For example, every test underneath ‘foo/tentative/’ will be considered tentative. " coming from http://web-platform-tests.org/writing-tests/file-names.html#test-features

Well, the assertions are required by the specification. Though the specification itself isn't mature yet, because there's no FPWD yet. So I guess I should put all the tests in a tentative/ folder, right?

Sebastian

[TalbotG]

So I guess I should put all the tests in a tentative/ folder, right?

Yes. Only the tests though. Not the reference files. Not the support files.

[TalbotG]

Your parsing tests:

http://www.gtalbot.org/BrowserBugsSection/CSS4BordersBoxDecorations/parsing/

Note that .js files are not at "/" but at "./" which is different from when tests are git-submitted and approved. I also appended "-SZ" to your tests. I need to double-check these tests, just in case I made a copy-&-paste error.

[TalbotG]

In parsing/box-shadow-color-computed.html at line 14, I see test_computed_value("box-shadow-color", "currentcolor", "rgb(255, 0, 0)"); but I can not find where the spec would suggest this. In my opinion, line 14 should be test_computed_value("box-shadow-color", "currentcolor", "rgb(0, 0, 0)");

"The initial value of color is black." coming from http://web-platform-tests.org/writing-tests/assumptions.html

[TalbotG]

In parsing/box-shadow-position-valid.html and in parsing/box-shadow-position-computed.html you can add test_valid_value("box-shadow-position", "outset, inset"); and test_computed_value("box-shadow-position", "outset, inset");

[TalbotG]

In parsing/box-shadow-spread-computed.html at line 18 test_computed_value("box-shadow-spread", "calc(1em + 1px)", "21px"); should be test_computed_value("box-shadow-spread", "calc(1em + 1px)", "17px");

---

In parsing/box-shadow-spread-valid.html at line 12 test_valid_value("box-shadow-spread", "0", "0px"); should be test_valid_value("box-shadow-spread", "0");

---

In parsing/box-shadow-offset-valid.html at lines 12 and 13, I believe that test_valid_value("box-shadow-offset", "0", "0px 0px"); test_valid_value("box-shadow-offset", "0 0", "0px 0px"); should be test_valid_value("box-shadow-offset", "0"); test_valid_value("box-shadow-offset", "0 0");

[TalbotG]

In parsing/box-shadow-blur-valid.html at line 12 test_valid_value("box-shadow-blur", "0", "0px"); should be test_valid_value("box-shadow-blur", "0");

[SebastianZ]

I think I've addressed all the issues and concerns. So @TalbotG @gsnedders, could you please have a look again?

Sebastian

[TalbotG]

Sebastian, I looked at your 2c6cc45 commit and at your a762890 commit and everything seems correct and fine. I will approve your Pull request as soon as @gsnedders Sam gives her Okay.

If later we find something wrong with a test or 2, we can then create another PR.

Gérard

[SebastianZ]

@gsnedders Did you already have time to look at this?

Sebastian

[TalbotG]

The only small doubt I have - and that is why I thought we should have a second opinion on this - is where the /tentative/ folder should be (assuming that it does matter). E.g. should the test files be moved to /css/css-borders/tentative/parsing/test-filename.html

or moved to

/css/css-borders/parsing/tentative/test-filename.html

It may even not matter anyway... but @gsnedders would know that for sure. I have never created tentative tests or use a /tentative/ folder .

Sebastian, your tests as far as I am concerned are good, worthy, correct, precise and compliant with all kinds of formating recommandations. I will wait a few more days and if we do not hear from Sam, then I will approve PR44525 .

Gérard

[TalbotG]

I approve the commits.

[TalbotG]

Sebastian,

If later there is a problem or complaint or issue with any of those 48 /parsing/ tests, then please let me know about it.

Gérard

[SebastianZ]

Thank you a lot for the reviews, help, and patience!

Sebastian