Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build spec links with short URLs for CSS testsuites #10248

Closed

Conversation

Projects
None yet
5 participants
@csnardi
Copy link
Member

csnardi commented Mar 29, 2018

Many spec links were in shortlink form (e.g. https://drafts.csswg.org/cssom/ instead of https://drafts.csswg.org/cssom-1/). However, build.py did not pick up these spec links, as it assumed that the only spec links could be in longhand form with the spec version. Update build.py to consider shortlinks when building.

Build spec links with short URLs for CSS testsuites
Many spec links were in shortlink form (e.g. https://drafts.csswg.org/cssom/ instead of https://drafts.csswg.org/cssom-1/). However, build.py did not pick up these spec links, as it assumed that the only spec links could be in longhand form with the spec version. Update build.py to consider shortlinks when building.
@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 29, 2018

Build PASSED

Started: 2018-03-29 23:37:00
Finished: 2018-03-29 23:40:52

View more information about this build on:

@csnardi

This comment has been minimized.

Copy link
Member Author

csnardi commented Mar 29, 2018

Hmm, this doesn't seem to fix it; not ready for review then.

@plinss

This comment has been minimized.

Copy link
Contributor

plinss commented Mar 29, 2018

@csnardi the linking to versioned rather than un-versioned spec links was a conscious design choice. The un-versioned URL changes over time to point to the latest spec where the test may have been written against an old version with different requirements.

If there are tests using un-versioned urls, then the tests should be changed rather than the build system.

Also note, the build will also currently reject spec links that don't have an anchor. If you're seeing tests fail to get picked up, that may also be a factor. There is a plan to accept tests that aren't linked to an anchor, but this will also require other changes since the rest of the tooling currently presumes that each test is linked to a specific section of the spec (derived form the anchor).

@csnardi

This comment has been minimized.

Copy link
Member Author

csnardi commented Mar 30, 2018

@plinss Certain places (e.g. the top of https://www.w3.org/TR/cssom-1/) link directly to unversioned editor's draft links. I unintentionally introduced a bunch of these into the cssom test suite in 825f054, as I followed that link and assumed it would just work.

I would also argue that it makes more sense for unversioned links to be applied to all specs with that short name, as CSS generally builds off of the previous versions of a spec. If behavior changes, that would seem to suggest that either the test should be dropped (as it's no longer useful), or that the spec link should be updated to explicitly state which versions it applies to. But in most cases, tests from prior versions should still be applicable to future versions, e.g. rgb tests from Color 3 should apply to Color 4.

@plinss

This comment has been minimized.

Copy link
Contributor

plinss commented Mar 30, 2018

But tests written for CSS Color 4 wouldn't work against a client that only implements CSS Color 3. If all the tests link to css-color, then there's no way to reason about them.

Also note that sometimes anchor IDs change between versions so a link to https://drafts.csswg.org/css-color-3/#foo may not link to https://drafts.csswg.org/css-color-4/ or https://drafts.csswg.org/css-color-5/ where the #foo anchor may not exist or potentially even refer to something else. (Yes, we try to keep anchors stable, but that doesn't always happen, or it may have moved to another css module for example, which is common.)

@plinss

This comment has been minimized.

Copy link
Contributor

plinss commented Mar 30, 2018

Also, the links to un-versioned draft links, for example the "Editors Draft" or "Latest Version" links are quite deliberate when we want to link people to the latest version. This is not to say that all links should be un-versioned, especially when referring to something in a specific version, like when testing a version of a spec.

@csnardi

This comment has been minimized.

Copy link
Member Author

csnardi commented Mar 30, 2018

I suppose then that this should be another lint, as I see more of these from other authors. You make valid points, but tooling should be helping authors, not making them do more work (especially when the easiest ways to find spec links results in intentional failure of the tools). This probably is relatively unimportant given the status of the CSS build system these days, though.

@csnardi csnardi closed this Mar 30, 2018

@csnardi csnardi deleted the csnardi:css-testsuite-short-urls branch Apr 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.