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

Add testing guideline for naming test files and adding comments #1664

Merged
merged 1 commit into from
Dec 7, 2017

Conversation

soareschen
Copy link
Contributor

I can add more about other aspects of testing later on. These are the two draft test guidelines that I come out with while still at TPAC.

We might also want to consider whether to put the main guidelines in webrtc-pc or web-platform-tests. There may be other contributors who work on web-platform-tests but not webrtc-pc. So it might be better off to put the guidelines in WPT and link to it from webrtc-pc.

@alvestrand
Copy link
Contributor

@fippo can you review this?
If you know of guidelines that cover parts of this that we should link to, please tell!

@fippo
Copy link
Contributor

fippo commented Nov 18, 2017

philip(p) confusion. Paging @foolip

@stefhak
Copy link
Contributor

stefhak commented Nov 30, 2017

@foolip could you take a look?

@alvestrand alvestrand requested a review from adam-be November 30, 2017 15:11
@burnburn burnburn self-requested a review November 30, 2017 15:11
@foolip
Copy link
Member

foolip commented Nov 30, 2017

It is really up to the people doing most of the work here to decide what amount of metadata and structure is useful. If I were to recommend anything, it would be to err on the side of less metadata to update, to optimize for ease of adding new tests.

Given the permission given to not bother with metadata in new tests or when updating tests, unless someone is continuously going to update the metadata, then it will get out of sync. As soon as that begins to happen, I think it might be better to delete the metadata.

@soareschen
Copy link
Contributor Author

@foolip yes agree we can remove a number of metadata from the individual test files, in particular the todo list. I think some coverage accounting is still essential until we reach 100% coverage. Some considerations are how essential are such metadata to others, and where should we put the metadata. Do we put them in web-platform-tests (web-platform-tests/wpt#8051)? or as unofficial forks (https://rwaldron.github.io/webrtc-pc/)? or in webrtc-pc?

@foolip
Copy link
Member

foolip commented Dec 4, 2017

@soareschen, all the attempts to determine coverage from test-to-spec linkage so far don't seem very successful. Linking to just sections isn't granular enough. If you search for "data-tested-assertations" you can see an attempt at something more granular dating back to web-platform-tests/wpt@6bb5305. It's a brittle approach, and I can only assume it's bitrotted.

The approach in speced/bikeshed#1116 is more promising to me, and should allow for views similar to https://rwaldron.github.io/webrtc-pc/. But that will be a Bikeshed feature, and WebRTC uses ReSpec. AFAICT, the least amount of work at this point would be to keep https://rwaldron.github.io/webrtc-pc/ up to date manually, by noting which wpt commit it's in sync with, and going through the changes since then for each update.

@soareschen
Copy link
Contributor Author

Updating the coverage can be pretty tedious. If not done incrementally, it could take a whole week to track changes to new editor's draft. I guess that means the best approach is to pull out the metadata and put them in unofficial forks. If the fork gets bitrotten, it is simply forgotten and affect neither web-platform-tests nor webrtc-pc.

is being tested. New tests may be written with minimal or no comments up to the
author's preference. If a test case is testing specific steps in a spec, it is
recommended to leave at least one line of comment referencing the spec to help
readers quickly understand the objective of the test.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to the "help link" that should point to the tested section in the spec?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention about "help link": web-platform-tests/wpt#8450 (comment)

@alvestrand alvestrand merged commit 4b4993f into w3c:master Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants