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
