I want to summarize what I've heard to facilitate some next steps and further discussion.
There seems to be broad support and no -1 votes to the idea of a soft check that tracks unit test coverage, so we wanted to get that out of the way. Daviddavis enabled unit test coverage reporting for all Pulp3 PRs ( https://github.com/pulp/pulp/pull/3397) and it will report on all PRs now. Currently, it shows 54.98% for pulpcore. That number is surprisingly high but not awesome. When looking at the report, it is mainly all import statements and function definitions since we have few/zero unit tests but also not much code. Based on feedback it sounds like leaving it at a soft check and highly encouraging unit tests with each PR is something we could all agree on. I want us to get to specific language that we can add into the Pulp3 docs as a new section called "Unit Tests" here: https://docs.pulpproject.org/en/3.0/nightly/contributing/index.html Here is a starting point, please send suggestions: "All new code is highly encouraged to have basic unit tests that demonstrate its functionality. A Pull Request that has failing unit tests cannot be merged." Also from the convo on-list and on-irc, here are some questions I would like help answering: * What areas in the existing codebase would really benefit from unit testing? I think we need a classpath list of modules and classes. I made an etherpad here: https://etherpad.net/p/Pulp_Unit_Test_Candidates * What are the existing unit tests and where do they live? * What docs need to be added to make contributing unit tests reasonable? Thanks for all the discussion! -Brian On Mon, Mar 26, 2018 at 4:01 PM, Jeremy Audet <[email protected]> wrote: > > I'm also generally -1 against trying to pick a number (100%, 80%, 60%) > up-front. We should unit test what makes sense to unit test, push that > number as high as reasonable, and otherwise focus on pulp-smash, which I > think has historically been more useful. > > QE is flattered by the focus on Pulp Smash. We're happy that the smoke > tests are being executed as a pull request check. > > However, it's important to remember that unit tests are far faster > than integration tests, typically by several orders of magnitude. In > addition, Pulp Smash's smoke tests are intentionally limited. They're > designed to execute quickly and to detect catastrophic regressions. > They're not intended to be comprehensive. In fact, some of the > already-written test cases may be stripped of their "smoke test" > status for the sake of speed. Psychology is important here: it's bad > if a developer locally fires off smoke tests, gets bored, and opens up > a new web browser tab. > > Please keep this limitation in mind when deciding on policies > regarding smoke tests. > > _______________________________________________ > Pulp-dev mailing list > [email protected] > https://www.redhat.com/mailman/listinfo/pulp-dev >
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
