search

opensearch-project / .github

Provides templates and resources for other OpenSearch project repositories.
Apache License 2.0
29 stars 70 forks source link

[PROPOSAL] Add guidelines for tests to maintainers.md file #55

Open bbarani opened 2 years ago

bbarani commented 2 years ago

What kind of business use case are you trying to solve? What are your requirements?

We need to define standards and guidelines around the tests for the contributions to improve the quality of the code.

What is the problem? What is preventing you from meeting the requirements? Currently, we dont have any guidelines around tests hence code contributions and merges are very subjective at this point in time

What are you proposing? What do you suggest we do to solve the problem or improve the existing situation? Suggest adding below section to https://github.com/opensearch-project/.github/blob/main/MAINTAINERS.md

Test the Changes

Check whether tests already exist in the project to test the contribution. If so, make sure to execute it every time before merging the changes. If the test doesn’t exist, understand the type of testing requirement ( Integration? Unit? Acceptance? ) for the contribution and see if we can re-use the framework by setting up a new testing harness for the project.

If adequate testing for a contribution does not exist, it is our responsibility to make sure the tests are added either by requesting the contributor to add one or more tests OR by adding the tests ourselves. If we are merging a fix, the added tests should fail without the fix. If we are merging an enhancement, we should make sure that the tests thoroughly exercise the new functionality.

What are your assumptions or prerequisites? Describe any assumptions you may be making that would limit the scope of this proposal.

What are remaining open questions? List questions that may need to be answered before proceeding with an implementation.

dblock commented 2 years ago

I like the spirit of this. I would rephrase it slightly and be shorter and more prescriptive. Particularly "we" and "us" are misused in the proposal and if it's directed at maintainers then it should say what the maintainer responsibilities are. Could be something like this:

Require Automated Tests

Provide and maintain continuous integration, test harnesses, and code coverage for the project. Ensure tests are executed for every contribution before code is merged and for every commit on main. Accept only pull requests with sufficient test coverage, including but not limited to unit, integration, and acceptance tests. Reject code that is insufficiently tested. Encourage improvements that increase the amount of automated testing, fill gaps in testing, improve test infrastructure and create GitHub issues that highlight gaps and ask the community for help in increasing the reliability of the project through automated testing.

Either way you should open a PR!