<div dir="ltr"><div><div>We would still block on failing tests, yes. </div>I'm also -1 blocking on coverage, and -1 against attempting 100%. </div>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. </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 26, 2018 at 12:31 PM, Bryan Kearney <<a href="mailto:bkearney@redhat.com" target="_blank">bkearney@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I assume you would still gate (hard fail) if unit tests fail? -- bk On 03/26/2018 05:55 AM, Ina Panova wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -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] <a href="https://media.giphy.com/media/g8GfH3i5F0hby/giphy.gif" rel="noreferrer" target="_blank">https://media.giphy.com/media/g8GfH3i5F0hby/giphy.gif</a> -------- 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 <<a href="mailto:austin@redhat.com" target="_blank">austin@redhat.com</a> <mailto:<a href="mailto:austin@redhat.com" target="_blank">austin@redhat.com</a>>> 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 <<a href="mailto:daviddavis@redhat.com" target="_blank">daviddavis@redhat.com</a> <div><div class="h5"> <mailto:<a href="mailto:daviddavis@redhat.com" target="_blank">daviddavis@redhat.com</a>>> 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 </div></div> *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] <a href="https://coveralls.io/" rel="noreferrer" target="_blank">https://coveralls.io/</a> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <mailto:<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a>> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> <<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a>> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <mailto:<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a>> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> <<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a>> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </blockquote><div class="HOEnZb"><div class="h5"> _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </div></div></blockquote></div> </div>