Re: Proposal to introduce test suite curators

Le 2017-03-03 01:33, Koji Ishii a écrit :
> What about requiring one implementer, instead of or in addition to,
> whichever we prefer?
> 
> When implementers are working on a feature, they are very motivated to
> review, and also they are very good reviewers since they look things 
> from
> different perspective.
> 
> I saw several cases where tests written before even the first impl are
> often not correct, even after reviewer's approval.

Then those several (!) tests should definitely be tagged as, reported as 
Incorrect in Shepherd system.

One annoyance with the Shepherd system is that any change to a test 
tagged as Incorrect will also change the category of the test from 
"Needs Work" to "Resubmitted for Review", even if the change is an 
unrelated change (say, the removal of a CR character).

> Years later, when someone try to implement, find questions to tests, or
> tests don't look correct, but test authors tend to forget what it was, 
> do
> not respond, or mail returned in error.

Lack of response - indeed - is a problem, has become a bigger problem 
and is one consequence of lack of review within a normal timeframe.

Somewhere someone (I can't remember when, who and in which mailing list) 
said that there is no reason why a reviewer should not make corrections 
to a (presumed incorrect) test that he did not author. I can not 
reconcile myself with such policy. Many years ago, in a documentation 
project, I remember making some harmless but reasonable changes in a 
file and this made the author angry, possibly very angry, at me. It is 
always preferable, best or better to seek and get the agreement, 
approval of the test author in the first place; in the process, this 
will help the test author to become better at authoring tests ... 
otherwise it will make the reviewer better at reviewing tests. Nobody is 
infallible.

> 
> So I think requiring an implementer can improve both review speed and 
> tests
> quality. WDYT?

Of course an implementer can improve both review speed and overall test 
quality. An implementer (software developer) has a perspective regarding 
a specification that a normal test author can not have.

Gérard

> 
> 2017-03-03 11:06 GMT+09:00 Florian Rivoal <florian@rivoal.net>:
> 
>> We suffer from a lack of review on tests. The average age of a PR on 
>> the
>> test repo is currently around 370 days, or 195 days even if we only 
>> count
>> those not marked "awaiting-submitter-response". That's way too long.
>> 
>> I think that's in part because nobody in particular feels responsible 
>> for
>> this.
>> 
>> I do not think we can force anyone to review tests if they'd rather be
>> doing something else, but we could work on identifying which spec lack 
>> test
>> reviewers, and try to find volunteers.
>> 
>> I think each spec needs a minimum of 2 test reviewers. They can be the
>> same as the editors, but do not have to be. I think it has to be at 
>> least
>> 2, because if there's only one, there's nobody to review that person's
>> tests, even though they are fairly likely to write some.
>> 
>> So I suggest we introduce a new role, which I'll call Test Curator 
>> (feel
>> free to bikeshed). The primary responsibility is not to write the 
>> entire
>> test suite, but to ensure that reviews and merges of Pull Requests get 
>> done
>> in a timely manner. Secondary responsibilities could include: being 
>> able to
>> identify which are of the test suite need more work (or to declare it
>> complete) or checking if existing tests are still valid after 
>> normative
>> changes in the spec.
>> 
>> I suggest that on each spec:
>> 
>> * we should list the "Test Curators" , next to and separately from the
>> Editors.
>> 
>> * Ask Editors if they're willing to be a Test Curator as well. If they
>> are, list them as such.
>> 
>> * For Every spec that has less than 2 test curators, call for 
>> volunteers
>> 
>> * Have the chairs periodically (monthly?) check if more people need to 
>> be
>> appointed (in addition or instead) for specs which still have a large 
>> queue
>> of Pull Requests. This should be take as seriously as finding new 
>> Editors
>> for a spec when the previous/current ones have left or aren't keeping 
>> up.
>> 
>> Note that this is separate from the (poorly named) OWNERS file used in 
>> the
>> web-platform-test repo, which is merely a notification mechanism, with 
>> no
>> implied responsibility.
>> 
>> —Florian
>>

Received on Friday, 3 March 2017 22:24:58 UTC