We would still block on failing tests, yes. I'm also -1 blocking on coverage, and -1 against attempting 100%.
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. On Mon, Mar 26, 2018 at 12:31 PM, Bryan Kearney <[email protected]> wrote: > I assume you would still gate (hard fail) if unit tests fail? > > -- bk > > On 03/26/2018 05:55 AM, Ina Panova wrote: > >> -1 for hard check, -1 for 100% coverage. >> >> Unittests are good but integration tests are better. >> >> I totally agree with what Austin said. We should add tests where it makes >> sense. +1 soft check. >> I would not like finding myself banging my head [0] (just because of 100% >> coverage policy) against one line of code to cover which is not really >> realistic to happen, +1 complex to mock. >> >> +1 to own policy of plugins. >> >> [0] https://media.giphy.com/media/g8GfH3i5F0hby/giphy.gif >> >> >> >> -------- >> Regards, >> >> Ina Panova >> Software Engineer| Pulp| Red Hat Inc. >> >> "Do not go where the path may lead, >> go instead where there is no path and leave a trail." >> >> On Fri, Mar 23, 2018 at 6:42 PM, Austin Macdonald <[email protected] >> <mailto:[email protected]>> wrote: >> >> -1 For a blocking check, -1 for attempting 100% coverage. >> >> There is a *lot* of code in Pulp 3 that simply involves inheriting >> from parents classes and defining attributes. If we add a ViewSet >> for instance, there is nothing to test other than "asserting that we >> did what we did". I have written lots of tests like that. IMO, they >> add no value and are time consuming to write. Also, they are time >> consuming to maintain because every change must also change the unit >> tests. This type of test almost never catches a real problem. >> >> A soft check would be a useful reminder to the contributor and the >> reviewer to add tests where they make sense. + 1 soft check >> >> Plugins: Each plugin team should determine their own unit test policy. >> >> >> On Fri, Mar 23, 2018 at 1:26 PM, David Davis <[email protected] >> <mailto:[email protected]>> wrote: >> >> We haven't had a unit test policy in Pulp 3, and the community >> and core committers would all like one. The general desire we've >> heard so far is to change course and encourage developers to add >> unit tests to their changes to Pulp 3. >> >> The policy we're suggesting is to add a coveralls[0] check for >> Pull Requests against the pulpcore 3.0-dev branch that shows the >> overall coverage percentage, e.g. 12.89%. This check would pass >> if and only if coverage increases or remains the same with the >> PR. We think this will eventually get us on the path to 100% >> unit test coverage. >> >> We propose the coveralls check be a soft check that allow for >> merging if it fails. We would document the policy and try to >> adhere to it even though it wouldn't formally block merging. At >> a future point when pulp3 (maybe the GA?) we could make this a >> hard check. >> >> Benefits: >> - It's easy, simple, and automatic >> - It's pretty objective and there's little room to argue with a >> number. >> - Helps us raise our unit test coverage gradually over time >> >> Downsides: >> - Could discourage community contributions >> - It can be a bit strict and unforgiving at times (especially if >> there's a hard check) >> - It only provides a guarantee around quantity of unit testing >> and not quality >> >> >> *Q: What about the existing functionality? Do we need to write >> unit tests for it?* >> >> We're not sure about this. We'd like community feedback. Is >> anyone interested in writing backfill unit tests? If so, then >> maybe we should. >> >> *Q: Will this also affect Pulp 2?* >> >> We're not planning on it but if we like this enough, we can look >> at adding it to Pulp 2. >> >> *Q: Will this affect plugins?* >> >> We want to start out with just pulpcore now and see how we like >> it. Then we'll look at adding it to pulp_file. In the future, we >> can also look at ways to make it easy for plugins to set this up. >> >> *Q: Do I no longer need to write pulp-smash test plan issues in >> Github for Pulp 3 features?* >> >> No, you should still do that. >> >> [0] https://coveralls.io/ >> >> _______________________________________________ >> Pulp-dev mailing list >> [email protected] <mailto:[email protected]> >> https://www.redhat.com/mailman/listinfo/pulp-dev >> <https://www.redhat.com/mailman/listinfo/pulp-dev> >> >> >> >> _______________________________________________ >> Pulp-dev mailing list >> [email protected] <mailto:[email protected]> >> https://www.redhat.com/mailman/listinfo/pulp-dev >> <https://www.redhat.com/mailman/listinfo/pulp-dev> >> >> >> >> >> _______________________________________________ >> 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 >
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
