[SSSD] Re: Every PR should come with a test.

[Jakub Hrozek]

On Wed, Jan 10, 2018 at 02:51:26PM +0100, Fabiano Fidêncio wrote:
> On Wed, Jan 10, 2018 at 2:28 PM, Jakub Hrozek  wrote:
> 
> > On Wed, Jan 10, 2018 at 10:52:56AM +0100, Sumit Bose wrote:
> > > On Wed, Jan 10, 2018 at 10:04:49AM +0100, Fabiano Fidêncio wrote:
> > > > People,
> > > >
> > > > Ideally every PR should come with a test (unit, integration, ...), but
> > > > unfortunately we're a little bit far from the ideal situation. Thus,
> > I'd
> > > > like to ask whether we have documented somewhere (apart from our code
> > > > itself) which are the parts of SSSD code that can be easily tested by
> > our
> > > > unit and integration tests.
> > > >
> > > > My understanding (and please, correct me if I'm mistaken) is that by
> > having
> > > > a updated list of our tests coverage would help any newcomer submitting
> > > > something new to the project and also not so experienced reviewers to
> > > > easily detect that a PR touching this or that part would need a test
> > > > (otherwise we don't even start reviewing the patches).
> > > >
> > > > So, does this list exist somewhere? Would be a fair request to create
> > this
> > > > list and have it linked to our "Contribute" page?
> > >
> > > iirc the CI scripts create coverage data. Would this help?
> >
> > I guess partially, because it looks like only unit tests (and not
> > integration tests) are generating the coverage.
> >
> > Also, I think this question comes from
> > https://github.com/SSSD/sssd/pull/476 and there the test would have to
> > be written in the IPA tree (there are already some tests for netgroups)
> >
> 
> Yes, the question comes from PR 476 but it's more general than that.
> IMO (and here I may be totally wrong) having a list of what we cover,
> what's covered by IPA, what's covered downstream would be useful to any
> *non* experienced reviewer (which is my case).

I see. What is covered by SSSD upstram tests is IMO best viewed in the
output of 'make check' and 'make intgcheck'. At least the test names
give an idea.

I don't have a good answer for IPA, I was usually just browsing the
files in the IPA git tree. And downstram is off-topic of this
list..
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Every PR should come with a test.

[Fabiano Fidêncio]

On Wed, Jan 10, 2018 at 2:28 PM, Jakub Hrozek  wrote:

> On Wed, Jan 10, 2018 at 10:52:56AM +0100, Sumit Bose wrote:
> > On Wed, Jan 10, 2018 at 10:04:49AM +0100, Fabiano Fidêncio wrote:
> > > People,
> > >
> > > Ideally every PR should come with a test (unit, integration, ...), but
> > > unfortunately we're a little bit far from the ideal situation. Thus,
> I'd
> > > like to ask whether we have documented somewhere (apart from our code
> > > itself) which are the parts of SSSD code that can be easily tested by
> our
> > > unit and integration tests.
> > >
> > > My understanding (and please, correct me if I'm mistaken) is that by
> having
> > > a updated list of our tests coverage would help any newcomer submitting
> > > something new to the project and also not so experienced reviewers to
> > > easily detect that a PR touching this or that part would need a test
> > > (otherwise we don't even start reviewing the patches).
> > >
> > > So, does this list exist somewhere? Would be a fair request to create
> this
> > > list and have it linked to our "Contribute" page?
> >
> > iirc the CI scripts create coverage data. Would this help?
>
> I guess partially, because it looks like only unit tests (and not
> integration tests) are generating the coverage.
>
> Also, I think this question comes from
> https://github.com/SSSD/sssd/pull/476 and there the test would have to
> be written in the IPA tree (there are already some tests for netgroups)
>

Yes, the question comes from PR 476 but it's more general than that.
IMO (and here I may be totally wrong) having a list of what we cover,
what's covered by IPA, what's covered downstream would be useful to any
*non* experienced reviewer (which is my case).


> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
>
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Every PR should come with a test.

[Jakub Hrozek]

On Wed, Jan 10, 2018 at 10:52:56AM +0100, Sumit Bose wrote:
> On Wed, Jan 10, 2018 at 10:04:49AM +0100, Fabiano Fidêncio wrote:
> > People,
> > 
> > Ideally every PR should come with a test (unit, integration, ...), but
> > unfortunately we're a little bit far from the ideal situation. Thus, I'd
> > like to ask whether we have documented somewhere (apart from our code
> > itself) which are the parts of SSSD code that can be easily tested by our
> > unit and integration tests.
> > 
> > My understanding (and please, correct me if I'm mistaken) is that by having
> > a updated list of our tests coverage would help any newcomer submitting
> > something new to the project and also not so experienced reviewers to
> > easily detect that a PR touching this or that part would need a test
> > (otherwise we don't even start reviewing the patches).
> > 
> > So, does this list exist somewhere? Would be a fair request to create this
> > list and have it linked to our "Contribute" page?
> 
> iirc the CI scripts create coverage data. Would this help?

I guess partially, because it looks like only unit tests (and not
integration tests) are generating the coverage.

Also, I think this question comes from
https://github.com/SSSD/sssd/pull/476 and there the test would have to
be written in the IPA tree (there are already some tests for netgroups)
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Every PR should come with a test.

[Sumit Bose]

On Wed, Jan 10, 2018 at 10:04:49AM +0100, Fabiano Fidêncio wrote:
> People,
> 
> Ideally every PR should come with a test (unit, integration, ...), but
> unfortunately we're a little bit far from the ideal situation. Thus, I'd
> like to ask whether we have documented somewhere (apart from our code
> itself) which are the parts of SSSD code that can be easily tested by our
> unit and integration tests.
> 
> My understanding (and please, correct me if I'm mistaken) is that by having
> a updated list of our tests coverage would help any newcomer submitting
> something new to the project and also not so experienced reviewers to
> easily detect that a PR touching this or that part would need a test
> (otherwise we don't even start reviewing the patches).
> 
> So, does this list exist somewhere? Would be a fair request to create this
> list and have it linked to our "Contribute" page?

iirc the CI scripts create coverage data. Would this help?

bye,
Sumit

> 
> Best Regards,
> -- 
> Fabiano Fidêncio

> ___
> sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
> To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org