[Avocado-devel] [RFC] Avocado Misc Tests repository policy

[Ademar Reis]

On Wed, Aug 03, 2016 at 10:07:01AM +0200, Amador Pahim wrote:
> Hello,
> 
> We are receiving a big number of Pull Requests in avocado-misc-tests
> repository. If on the one hand it is really great to see the community
> using and contributing to Avocado, on the other hand the Avocado devel
> team should not be that involved in review code of tests, since our
> business is to write Avocado itself.
> 
> This RFC aims to outline the workflow of the Avocado devel team
> regarding the avocado-misc-tests repository.
> 
> Motivation
> ========
> The code review is a time consuming activity and review code of tests
> is not part of the Avocado devel team business. Given the high number
> of Pull Requests on avocado-miscs-tests repository, we need a policy
> to officially state our participation there.

I think we need to take a step back and clarify the purpose of
the avocado-misc-tests repository. It was born as a way to port
the autotest client tests to Avocado, but since then, it's
purpose morphed into something else.

Do we want all kinds of tests in avocado-misc-tests? Should it be
a reference repository, where only high-quality tests are merged,
serving as an example for other test writers? Should the tests
there work with avocado sprint releases, or LTS? In other words,
what should be the requirements to have a test added to
avocado-misc-tests?

Personally, I think each team, company of person involved in
testing with Avocado should have their own test repository(ies),
upstream or not (allowing this is one of the primary design goals
of Avocado).

Then we would have a few options for what to do with
avocado-misc-tests:

Option 1: avocado-misc-tests becomes a reference repository of
high-quality, example tests.  In order to be accepted there,
tests should pass the following requirements:

   a) Useful to a wider community (not a single project or
   company).
   b) Well written, using proper Avocado APIs and features.
   c) Maintained: tests that break or don't catch up to changes
   in avocado or other dependencies should be quickly removed.
   d) Mature and proven stable: which means they've been used in
   production for a certain amount of time (3 months?) before
   getting "promoted" to avocado-misc-tests.

Even with all these requirements, there's no way we can guarantee
the quality or stability of the tests there without more
maintainers stepping up.

As of now, avocado-misc-tests is a (very) low priority to the
core Avocado team, which means in practice it might turn into
what's described by option 2 below:

Option 2: we accept any kind of test to avocado-misc-tests, as
long as it's reviewed by somebody, in a semi-automated fashion.
In other words, it should be considered a unstable repository of
tests, without any guarantee or expectations about code-quality
or stability. A kind of wild-wild-west, use-at-your-own-risk
repository.

Option 3: port Autotest client tests to it, but don't accept any
other tests.

Option 4: Declare it unmaintained (to be removed from the Avocado
umbrella unless someone volunteers to maintain it). Its future
will depend on the efforts of the new maintainers and the quality
of what gets merged there (only time will tell).

Thoughts?

Thanks.
   - Ademar

> 
> Proposal
> =======
> We don't believe that we should just not to look to the
> avocado-misc-tests repository. A good number of bugfixes and features
> in Avocado were born as consequence of tests posted there.
> 
> To find a balance, the initial proposal is that the Avocado devel
> team, currently the only maintainers of that repository, will only get
> involved after a third party code review and ACK of a given Pull
> Request. That way, the code author should be also in charge of find
> someone to review its code, being the reviewer from the same company
> or not. We can always invite people to review code there, but it's
> essentially the author's responsibility.
> 
> For the reviewer, it is expected that he/she:
> - Reads the code, commenting with suggestions of improvements: good
> practices, general standards, effectiveness of code, verify comments
> and docstrings.
> - Test the code: run the test and make sure it's working as expected.
> - Ping the authors of Pull Requests already reviewed and not updated
> for a long time.
> - When the code is considered ready, comment the Pull Request with an
> 'Looks Good To Me'.
> 
> The 'Looks Good To Me' comment will be the trigger for the maintainers
> to go there and take a final look on the Pull Request and merge it.
> 
> Expected Results
> ==============
> The expected result is to decrease the load of Avocado devel team in
> regards to the code review in that repository.
> Another important expected outcome of this process is to give the
> merge permission for (aka promote to maintainer) those assiduous
> reviewers with good quality of reviews.
> When this RFC is considered ready, we will update our documentation
> and the avocado-misc-test README file to reflect the information.
> 
> Additional Information
> ================
> Any individual willing to make the code review is eligible to do so.
> And the process is simple. Just go there and review the code.
> Given the high volume of code coming from IBM, I had a chat with
> Praveen Pandey, an IBMer and assiduous author of Pull Requests for
> avocado-misc-tests, and he agreed in make reviews in
> avocado-misc-tests.
> 
> 
> Looking forward to read your comments.
> --
> apahim
> 

-- 
Ademar Reis
Red Hat

^[:wq!