[Pulp-dev] Pulp 3 Unit Test Plan Proposal

Ina Panova ipanova at redhat.com
Mon Mar 26 09:55:12 UTC 2018


-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 <austin at 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 <daviddavis at 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 at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180326/4abe92a7/attachment.htm>


More information about the Pulp-dev mailing list