Re: [openstack-dev] [horizon] Minimum Unit Test Coverage
I was referring to the HTML reports that the karma-coverage plugin creates for now. From my experience with it, it’s fairly relaxed about what counts as something being tested, hence the 100% aim. For example, often just checking that a value is defined is enough for it to be “tested”, and this is where reviewers would have to use their own knowledge to ensure decent tests. More than happy to discuss tooling though. Rob From: Michael Krotscheck mailto:krotsch...@gmail.com>> Reply-To: "OpenStack Development Mailing List (not for usage questions)" mailto:openstack-dev@lists.openstack.org>> Date: Thursday, 23 July 2015 18:11 To: "OpenStack Development Mailing List (not for usage questions)" mailto:openstack-dev@lists.openstack.org>> Subject: Re: [openstack-dev] [horizon] Minimum Unit Test Coverage +1 on coverage of any kind. >From a tooling perspective, are you thinking istanbul? >From an infra perspective, are you thinking a separate job, or to have it >integrated in with npm run test? FYI- istanbul wraps the unit test invocation, >e.g. 'istanbul karma start ./karma.config.js' or something similar. 100% code coverage is ambitious. Let's get the tool selected and working first. Michael On Wed, Jul 22, 2015 at 11:57 AM Rajat Vig mailto:raj...@thoughtworks.com>> wrote: Hi Rob I agree. Enforcing a minimum level of coverage as a start is awesome. I must add though keeping it at 100% and breaking the build has almost never worked in practice for me. Keeping a slightly lower level ~98% is slightly more pragmatic. Also, the currently low coverages will have to be addressed as well. Is there a blueprint that can be created to tackle it? -Rajat On Wed, Jul 22, 2015 at 6:33 AM, Rob Cresswell (rcresswe) mailto:rcres...@cisco.com>> wrote: Hi all, As far as I’m aware, we don’t currently enforce any minimum unit test coverage, despite Karma generating reports. I think as part of the review guidelines, it would be useful to set a minimum. Since Karma’s detection is fairly relaxed, I’d put it at 100% on the automated reports. I think the biggest drawback is that the tests may not be “valuable”, but rather just meet the minimum requirements. I understand this sentiment, but I think that “less valuable” is better then “not present” and it gives reviewers a clear line to +1/ -1 a patch. Furthermore, it encourages the unit tests to be written in the first place, so that reviewers can then ask for improvements, rather than miss them. Rob __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe<http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe<http://openstack-dev-requ...@lists.openstack.org?subject:unsubscribe> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [horizon] Minimum Unit Test Coverage
+1 on coverage of any kind. >From a tooling perspective, are you thinking istanbul? >From an infra perspective, are you thinking a separate job, or to have it integrated in with npm run test? FYI- istanbul wraps the unit test invocation, e.g. 'istanbul karma start ./karma.config.js' or something similar. 100% code coverage is ambitious. Let's get the tool selected and working first. Michael On Wed, Jul 22, 2015 at 11:57 AM Rajat Vig wrote: > Hi Rob > > I agree. Enforcing a minimum level of coverage as a start is awesome. > > I must add though keeping it at 100% and breaking the build has almost > never worked in practice for me. > Keeping a slightly lower level ~98% is slightly more pragmatic. > Also, the currently low coverages will have to be addressed as well. > Is there a blueprint that can be created to tackle it? > > -Rajat > > > On Wed, Jul 22, 2015 at 6:33 AM, Rob Cresswell (rcresswe) < > rcres...@cisco.com> wrote: > >> Hi all, >> >> As far as I’m aware, we don’t currently enforce any minimum unit test >> coverage, despite Karma generating reports. I think as part of the review >> guidelines, it would be useful to set a minimum. Since Karma’s detection is >> fairly relaxed, I’d put it at 100% on the automated reports. >> >> I think the biggest drawback is that the tests may not be “valuable”, >> but rather just meet the minimum requirements. I understand this sentiment, >> but I think that “less valuable” is better then “not present” and it gives >> reviewers a clear line to +1/ -1 a patch. Furthermore, it encourages the >> unit tests to be written in the first place, so that reviewers can then ask >> for improvements, rather than miss them. >> >> Rob >> >> __ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: >> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [horizon] Minimum Unit Test Coverage
Hi Rob I agree. Enforcing a minimum level of coverage as a start is awesome. I must add though keeping it at 100% and breaking the build has almost never worked in practice for me. Keeping a slightly lower level ~98% is slightly more pragmatic. Also, the currently low coverages will have to be addressed as well. Is there a blueprint that can be created to tackle it? -Rajat On Wed, Jul 22, 2015 at 6:33 AM, Rob Cresswell (rcresswe) < rcres...@cisco.com> wrote: > Hi all, > > As far as I’m aware, we don’t currently enforce any minimum unit test > coverage, despite Karma generating reports. I think as part of the review > guidelines, it would be useful to set a minimum. Since Karma’s detection is > fairly relaxed, I’d put it at 100% on the automated reports. > > I think the biggest drawback is that the tests may not be “valuable”, > but rather just meet the minimum requirements. I understand this sentiment, > but I think that “less valuable” is better then “not present” and it gives > reviewers a clear line to +1/ -1 a patch. Furthermore, it encourages the > unit tests to be written in the first place, so that reviewers can then ask > for improvements, rather than miss them. > > Rob > > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [horizon] Minimum Unit Test Coverage
Hi all, As far as I’m aware, we don’t currently enforce any minimum unit test coverage, despite Karma generating reports. I think as part of the review guidelines, it would be useful to set a minimum. Since Karma’s detection is fairly relaxed, I’d put it at 100% on the automated reports. I think the biggest drawback is that the tests may not be “valuable”, but rather just meet the minimum requirements. I understand this sentiment, but I think that “less valuable” is better then “not present” and it gives reviewers a clear line to +1/ -1 a patch. Furthermore, it encourages the unit tests to be written in the first place, so that reviewers can then ask for improvements, rather than miss them. Rob __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
