-
Notifications
You must be signed in to change notification settings - Fork 201
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
Inline WPT test information into specs #1116
Comments
Issue: Sometimes we do combinatorial testing of a feature, which can result in huge numbers of tests for a single line. If these use predictable naming scheme, maybe I can have |
I don't think that automated typo-fixing would be that useful - probably better to require the actual test name so that there isn't any confusion or need to reverse-engineer the code to locate a test. |
Multiple (versioned) specs will share the same test folder. So it shouldn't be an error that a level 3 draft isn't including a level 4 test in its unversioned test folder. |
Oh, definitely not typo-fixing. Just typo-finding, so I can report suggestions in the error, like I do today for biblio link typos. |
This sounds pretty great!! One thing I don't see addressed here is the cost to the spec author to continually monitor the appropriate directory and add appropriate annotations to their source spec as new tests flow in. This burden seems unavoidable on someone, and placing it on the spec author is IMO better than placing it on the test writer. But I think it's worth keeping in mind that the burden exists. |
So the convention is to add the annotation after the assertion. Perhaps we could have a bulk-import option to automatically add these to the ends of sections based on the test metadata links (if present)? This could come with an additional note (automated-entry) so the spec editor could move it to a more specific location within the section. This could be used the first time you added the annotations to the spec, and/or each time a large number of new tests needed to be accounted for. |
Check the fifth bullet-point in the task-list - Bikeshed knows the current state of WPT (if you've run
Might be reasonable. I can still throw a warning while automated-placement tests exist, reminding you to move them to the correct place, but at least letting you opt into the prefix-watching immediately. |
This seems like it'll be particularly burdensome whenever there's a large number of new tests all being written more or less at once (either because of mass-upstreaming, or during initial implementation). A few other thoughts:
|
That's definitely a hard problem, and I'm not sure what to do about it. I'm wondering if part of the reason for big single-page tests is because it's annoying to do extra tracking and reviewing for multiple tests? Maybe this (and further improvements) can shift the rewards here.
This issue would at least give us better targetting of the boxes. Haven't given any thought to how it would interact, tho. We could run with the same basic design as the CanIUse panels that Bikeshed currently adds, which are as up-to-date as the last time you ran
Yes, I think so. The plan already has me gathering the title/assert from the test file; I can grab more metadata as needed and do whatever linting is useful based on it. (I might, btw, be putting a bug on y'all to track this info in the manifest file instead, so I don't have to parse everything on my own, and others can get the same info.)
Right, Shepherd (or another service that knows about the set of Bikeshed specs) can parse the HTML of the with-tests output file, and automagically get the data out. This doesn't seem particularly hard. |
This is very exciting! @rwaldron, given your work on inline annotations in https://rwaldron.github.io/webrtc-pc/, WDYT about this approach? Taking https://fullscreen.spec.whatwg.org/ as an example, there is something I imagine I'd quickly want, namely to associate https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen with fullscreen/api/element-request-fullscreen-*.html (wildcard) in wpt. As for presenting the tests, it would be pretty great to have fresh results from wpt.fyi show up. @lukebjerring, you added an endpoint for the latest test results recently, can you point us to that? |
Explicitly a non-goal. The point is to put the tests next to the text they're testing. Wildcarding like that is basically equivalent to the existing |
Currently https://wpt.fyi/latest/{platform}/[test path] redirects to JSON for the test run. This may change with web-platform-tests/results-collection#161 and web-platform-tests/results-collection#172 being resolved. |
Desirable syntaxes and outputs, from @fantasai: Compact
Annotations
Table of related tests
|
Even something relatively small like https://github.com/w3c/web-platform-tests/blob/master/dom/nodes/Node-isEqualNode.html could be problematic. It seems overkill to me to split it up into separate files for each different case in https://dom.spec.whatwg.org/#concept-node-equals (and IMO would be much less maintainable as a testsuite like that given you'll constantly be opening and closing new files looking at very similar but very slightly different tests). Do we know how non-CSS spec authors feel about this? Do they want to move over to having such fine-grained metadata at all, or does the the current directory structure approach suffice? It's only the CSS WG that seems to have much problem with looking at test coverage, and I don't know if that means the CSS WG care far more about test coverage than any other spec (which seems unlikely given the relatively small test suites) or if there's something else different. |
I... strongly doubt that. There are no automated coverage trackers for specs, so I'm nearly certain that "doesn't have a problem with" actually means "doesn't really know what their test coverage is". The current method for determining coverage is annoying and labor-intensive, so I doubt that's done basically ever; at best, people are relying on "I wrote a pretty comprehensive suite at Time N, and since then I've required tests for every new addition"—a "dead reckoning" style of coverage tracking vs navigating based on actual information. |
That's fine, it's a single small dl, put all of it's tests together. Or split them up per dt/dd pair, that's easy too, it's like five cases, and you won't have to remember to add a new test if you add a new case (because the lack of test will be obvious). |
Is
The Does that make sense? |
@foolip sorry, somehow I missed your mention. This definitely sounds interesting to me and I'd like to "test drive" the idea on some existing spec content to see how the authoring flow works. |
Simple implementation on the ReSpec side (look for "tests: 1"): It just uses:
And uses test suite URL from config. Will see what we need to align with this proposal. |
I'm super excited about this - adding myself to this thread to keep track of changes. |
Some feature suggestions for v2:
@fantasai as the source of some of these suggestions, since they wanted to be subscribed to the thread ^_^ |
Whoa, how did I miss this?!? By the looks of this - it looks like it's a new prop at the top of the bs file pointing to the central w3 test suite (eg: http://w3c-test.org/css/css-flexbox/) and then it grabs all of the test info? I agree with @fantasai that I want inline testing as that lets you know whether you do or don't have a test for specific line of test. The above accomplishes similar capabilities to the script I wrote (although having it in the build process is good) which gives you a rough idea of the counts but it won't let you know if a specific line is tested (unless I'm missing something in the code - which is possible ;) ) |
@gregwhitworth I recommend reading the documentation at https://tabatkins.github.io/bikeshed/#testing; by "inline" I mean as in "inline element in the middle of a sentence". Putting the tests in particular parts of the spec is already supported (and explicitly encouraged), it just makes a block element. ^_^ |
@tabatkins beautiful - thanks! |
side note here: w3c-test.org isn't meant to be a stable server since it's running wptserve. If we link it more from specs, we should make suire we're ok with the server crashing from time to time. This would contradict w3c/specberus#758 btw. |
I think it would be great if it was possible to determine test coverage for specs using this feature. I'm not sure what is needed to make that possible, but maybe a way for tests to be explicitly associated with a piece of spec text (e.g. instead of a |
Well, everything in a spec is normative, unless it's a note, issue, figure, etc. or explicitly marked as informative. Thus, it's theoretically possible to identify untested sections - particularly algorithms - in a spec (of course, YMMV). |
You can also have statements of fact and definitions, which may not need any tests. But maybe it's possible to detect those (check for lack of RFC 2119 keywords, check for |
Detail: once I have subtest-addressing, how should I react when you link to some of the subtests but not all? It was pointed out in the ecosystem-infra call that there shouldn't be a semantic difference between subtests-in-one-file and tests-in-one-folder; which you use is just an implementation difference based on which is easier in a single case. The two cases should be handled parallel. So, since I think it's reasonable to be able to say "all of the subtests in this file are for this particular feature" (and have Bikeshed automatically expand it to all the subtests), it sounds also reasonable to be able to say that "all of the tests in this folder are for this particular feature" (and have Bikeshed automatically expand). This'll be similar to (This potentially suffers from the "later people put more tests in this folder/file that aren't meant to show up at this spot in the spec", but I figure it's useful enough that it overrides this downside.) |
This change adds initial support for the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. This change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, http://web-platform-tests.live, and https://wpt.fyi URLs for the listed tests. The change doesn’t provide the following `<wpt>`-related features: - Doesn’t yet verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t yet verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This change adds initial support for the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. This change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, http://web-platform-tests.live, and https://wpt.fyi URLs for the listed tests. The change doesn’t provide the following `<wpt>`-related features: - Doesn’t yet verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t yet verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This change adds initial support for the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. This change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, http://web-platform-tests.live, and https://wpt.fyi URLs for the listed tests. The change doesn’t provide the following `<wpt>`-related features: - Doesn’t yet verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t yet verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This change adds initial support for the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. This change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, http://web-platform-tests.live, and https://wpt.fyi URLs for the listed tests. The change doesn’t provide the following `<wpt>`-related features: - Doesn’t yet verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t yet verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This change adds initial support for the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. This change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, http://web-platform-tests.live, and https://wpt.fyi URLs for the listed tests. The change doesn’t provide the following `<wpt>`-related features: - Doesn’t yet verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t yet verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This change adds support for generating output from the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. Specifically, this change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, https://wpt.fyi and https://web-platform-tests.live URLs for the listed tests. The change by design intentionally doesn’t provide the following `<wpt>`-related features: - Doesn’t verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87
This adds support for generating output from the `<wpt>` element, as documented at https://tabatkins.github.io/bikeshed/#wpt-element) and discussed at speced/bikeshed#1116. Specifically, this change causes lists of tests in `<wpt>` elements from the source to generate TESTS sections in the spec output, with links to corresponding https://github.com/web-platform-tests/wpt, https://wpt.fyi and https://web-platform-tests.live URLs for the listed tests. The change intentionally doesn’t provide the following `<wpt>`-related features: - Doesn’t verify that each test listed in a `<wpt>` element actually exists in https://github.com/web-platform-tests/wpt/ - Doesn’t verify that every single test file that exists in the https://github.com/web-platform-tests/wpt/tree/master/html tree is listed in a `<wpt>` element somewhere in the spec source. Fixes #87.
Unclear why this issue is still open, the proposed feature is implemented and in widespread use. Close? |
Background
Talking with fantasai, a major hurdle she has with dealing with testing is figuring out what is un/undertested. WPT has a view that at least sorts tests by ToC section, but that's not enough - you still have to manually read all of the tests and reverse them to testable statements in the spec yourself, annotating your personal copy, and then look at what you have and what wasn't touched. Better would be to have test information inline in the spec, where after each paragraph (or even sentence!) you list the tests associated with that bit of text. This gives you an immediate view in the source of what's untested, and makes it easy to keep the tests in the right place when you move stuff around in the spec.
Benefits / Drawbacks
Why Hasn't This Been Done Before?
Very few people have ever worked on the spec-authoring side of the toolchain before; in recent memory, only Wattsi (very HTML-specific) and ReSpec (can't handle large quantities of data, like the test db) have competed with Bikeshed. Most people working on test-tooling, then, operate from the assumption that the specs are unchangeable, and direct their engineering efforts accordingly to work around specs and link into them, rather than having the specs link out. As such, I think it's simply been a matter of no one ever trying to work on it from this side.
Rough Design
wpt manifest
to get the list of paths. Stored inspec-data/wpt-tests.txt
.)<wpt>foo/bar.html</wpt>
element to Bikeshed. Takes test names/paths as text content, one test per line. Does not show up in the final document by default. (Done.)<test>
to let you override that, if for whatever reason you need to. Path is concatenated with the testname (with intervening/
if necessary) to locate the actual test. (Append to the prefix on a per-section/container basis, to let you specialize to a subfolder?) (Done, viaWPT Path Prefix
metadata, orpathprefix
attribute on<wpt>
.)Collect all the test's titles and asserts, which isn't currently collected by the manifest.<a title="test's <title> and assert" href="link to test runner for test">test name</a> <a href="link to test source">...</a>
. (Done - ifWPT Display: inline
is set,<wpt>
becomes a Tests block that lists the tests with several useful related links.)The text was updated successfully, but these errors were encountered: