Sonar test coverage inaccuracy

[Paweł Albecki]

Hi all,

During walk-through SonarQube unit test coverage tool for CCE, I discovered that there is inaccuracy in code line coverage. For instance, if we take /cce/service/notifier package, we can see that there is 89.2% coverage or /cce/web/csv/processor with 92.5% which seems fine but If I take a look at /cce/web package we can see that there is 0% coverage. Since currently the most logic is in our controllers, general CCE service coverage looks poor – 21.6%. As you can see at CCE GitHub repository, we have many integration tests that covers edge cases from controllers as well as some code from csv parsers, domain and dto (like newInstance method or just fields). The same story is for repository which is covered in integration tests but Sonar show 0%. Can we resolve this issue somehow? 

On Sonar, we can also see that some utils and services that are copy-pasted from other services has lack of unit tests. I think that we should resolve this by copy and paste tests for these classes from other services or move these classes to our openlmis-service-util that was not updated for a long time. I have in mind classes that we can find in /cce/util package and some from /cce/service package (like AuthService or BaseCommunicationService classes). What do you think?

Regards,

Paweł

[Josh Zamor]

Thanks Pawel,

I believe what you’re seeing is intentional - it’s reporting on unit coverage.  If you look at our Testing Guide we have a number of different kinds of tests.  Somewhere in that stack of different types of tests we might have a test which “covers” some line of code, but which isn’t included in the Sonar metric. This is the right thing to do when that test is not a unit test.  Unit tests in the testing pyramid should be our go to type of test.  As we’ve discussed at length here and in various calls, we should have the most of these types of tests.

When the only logic we have is in controllers, that’s potentially concerning as we generally want business logic in domain classes.  When those controllers have logic, and that logic is not covered by a unit test, only some other type of test (such as an integration test), that’s a red flag, and Sonar is helping us identify that.

We want business logic to be unit tested.  We want other types of tests as well, such as integration tests and contract tests, however our go to one metric view of testing reflecting on a healthy code-base is good unit test coverage.

As for the utility classes, I believe developers have not added to that due to those utility classes being Spring dependent.  Our services have different versions of Spring Boot, and there is a concern of unforeseen coupling between those services if we have a shared dependency (library) that has a basis in one particular spring version.  I’m open to suggestions for reducing duplication that solves the dependency concern.  In the meantime, we should certainly be copying tests for a utility class if we’re already copying the utility class.

Best,

Josh

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.
To post to this group, send email to openlm...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/49c8910c-2123-4535-b51e-697666cece3f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Paweł Albecki]

By logic I meant mainly some calls to other services or repository which can't really be part of domain object. We don't have much business logic in CCE service so domain objects looks poor.

If we don't want to include IT in Sonar, shouldn't web package be disabled from unit test coverage inspection?
About utility classes, I think someone forgot to copy tests for them during creation of CCE service. I will create some ticket to fix that gap.

Regarding reducing duplication, I will take a deeper look if we can move some non Spring dependent classes to service util library.

Regards,
Paweł

[Paweł Albecki]

Good candidates for service util library:

RequestParameters, JaVersDateProvider, Message (after get rid of Spring's MessageSource), many Exception classes like BaseMessageException, ValidationMessageException etc.

On Wednesday, September 13, 2017 at 6:42:31 PM UTC+2, Paweł Albecki wrote:

[Paweł Gesek]

Josh - pretty sure it isn't intentional, in all of our services we have this line:

https://github.com/OpenLMIS/openlmis-cce/blob/master/build.gradle#L202

which includes integration tests in Sonar, at least that's what it is supposed to do.

It seems that this works for some services, and doesn't for others. For example Stock Management has 82% coverage in the web package, but all I see for that package are integration tests:

https://github.com/OpenLMIS/openlmis-stockmanagement/tree/master/src/integration-test/java/org/openlmis/stockmanagement/web

I don't see any unit tests for that package:

https://github.com/OpenLMIS/openlmis-stockmanagement/tree/master/src/test/java/org/openlmis/stockmanagement/web

Unless I am missing something, seems our code coverage reports might be a bit skewed between services. We should unify the measure, whether we want ITs in it or not.

Regards,

Paweł

SolDevelo Sp. z o.o. [LLC] / www.soldevelo.com
Al. Zwycięstwa 96/98, 81-451, Gdynia, Poland
Phone: +48 58 782 45 40 / Fax: +48 58 782 45 41

--
You received this message because you are subscribed to the Google Groups "OpenLMIS Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev+unsubscribe@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/237ddb8d-8630-4f47-9d7f-fbf38e08d0d6%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Paweł Gesek
Technical Project Manager
pge...@soldevelo.com / +48 690 020 875

[Josh Zamor]

Thanks Pawel G and Pawel A,

You’re right.  I was going to say that perhaps it’s the one project that’s been re-configured in Sonar, however in the new version of Sonar they’ve dropped support for custom project dashboards.

Feel free to add some tickets to fix to the Platform or component backlog.  As I have free time I’ll try to see what I can do as well, though clearly Sonar has some new ways of doing things that I’ll need to learn. 

Best,

Josh

To unsubscribe from this group and stop receiving emails from it, send an email to openlmis-dev...@googlegroups.com.

To post to this group, send email to openlm...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/openlmis-dev/CADt-Nu0wVJvXd6CbU3yhfWUPaEYyRWBr5eswZtS%3DoH-HDrj3xw%40mail.gmail.com.

[Paweł Albecki]

Josh,  

As for the utility classes, I believe developers have not added to that due to those utility classes being Spring dependent.  Our services have different versions of Spring Boot, and there is a concern of unforeseen coupling between those services if we have a shared dependency (library) that has a basis in one particular spring version.  I’m open to suggestions for reducing duplication that solves the dependency concern.

Can't shared library have versions that support particular Spring Boot version? In that way if some service has older version of Spring Boot, it will use older version of Service Util.

On Wednesday, September 13, 2017 at 6:42:31 PM UTC+2, Paweł Albecki wrote:

[Paweł Albecki]

Hi again,

to close this topic I have to say that we recently fixed test coverage inaccuracy (all our services include integration tests in test coverage) and the discussion about shared library should be continued here: https://groups.google.com/forum/#!topic/openlmis-dev/qagrYjk8ghs

Thanks,

Paweł

On Wednesday, September 13, 2017 at 6:42:31 PM UTC+2, Paweł Albecki wrote: