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 <bkear...@redhat.com> 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 <aus...@redhat.com
>> <mailto:aus...@redhat.com>> 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 <davidda...@redhat.com
>>     <mailto:davidda...@redhat.com>> 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
>>         Pulp-dev@redhat.com <mailto:Pulp-dev@redhat.com>
>>         https://www.redhat.com/mailman/listinfo/pulp-dev
>>         <https://www.redhat.com/mailman/listinfo/pulp-dev>
>>
>>
>>
>>     _______________________________________________
>>     Pulp-dev mailing list
>>     Pulp-dev@redhat.com <mailto:Pulp-dev@redhat.com>
>>     https://www.redhat.com/mailman/listinfo/pulp-dev
>>     <https://www.redhat.com/mailman/listinfo/pulp-dev>
>>
>>
>>
>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev@redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev@redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
_______________________________________________
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

Reply via email to