Fixes bullseye conditions to be shown in SQ UI #1237

tomassdepakin · 2017-09-22T12:28:39Z

Hi. Fixing bug, when bullseye conditions are not shown in SonarQube UI.
Tested with 6.3

Removing unneeded CoverageType from CoverageMeasure. It successfully contains line hits and conditions simultaneously.

Here comes my paint madskillz:

This change is

Bertk · 2017-09-23T06:37:55Z

👍 Thanks for this improvement. Did you also check it for SQ 5.6?
I could not find until now any reason why this should not work with SQ 5.6.
http://javadocs.sonarsource.org/5.6/apidocs/org/sonar/api/batch/sensor/coverage/NewCoverage.html

We missed this information (https://docs.sonarqube.org/display/DEV/API+Changes):

DEPRECATED NewCoverage::ofType is deprecated and ignored. It is possible to feed as many coverage reports as you want for the same file. They will be merged without keeping any distinction of their "type".

Bertk · 2017-09-23T06:39:59Z

cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CoverageMeasures.java

-import java.util.Map;
-import java.util.Set;
-import java.util.Map.Entry;
+import java.util.*;



squid:S2208 Wildcard imports should not be used

Bertk · 2017-09-23T06:45:44Z

cxx-sensors/src/main/java/org/sonar/cxx/sensors/coverage/CxxCoverageSensor.java

    try {
-      if(type == CoverageMeasure.CoverageType.CONDITION) {
+        newCoverage.lineHits(measure.getLine(), measure.getHits());


squid:IndentationCheck Make this line start at column 7

Bertk · 2017-09-23T07:19:42Z

The integration tests failed because the coverage values for SQ 5.6 changed (Travis and AppVeyor).

Feature: Importing coverage data
  @SqApi56
  Scenario: Importing coverage reports
      Assertion Failed: 
        overall_line_coverage is actually 28.2 [expected: 20.0]
        line_coverage is actually 17.6 [expected: 12.5]
        it_line_coverage is actually 36.4 [expected: 26.3]
        coverage is actually 23.8 [expected: 20.0]
        overall_coverage is actually 34.0 [expected: 28.6]
        it_coverage is actually 40.6 [expected: 34.5]
        
  @SqApi56
  Scenario: Importing coverage reports zeroing coverage for untouched files 
      Assertion Failed: 
        overall_line_coverage is actually 20.0 [expected: 13.7]
        line_coverage is actually 7.3 [expected: 5.0]
        it_line_coverage is actually 19.0 [expected: 12.8]
        coverage is actually 11.1 [expected: 9.1]
        overall_coverage is actually 26.1 [expected: 21.5]
        it_coverage is actually 25.0 [expected: 20.4]
        

Feature: Smoketest
  @SqApi56
  Scenario: Smoketest
      Assertion Failed: 
        overall_coverage is actually 84.0 [expected: 81.8]
        it_coverage is actually 84.0 [expected: 81.8]
        coverage is actually 84.0 [expected: 81.8]

I found a comment in test_create_rules_and_bullseye_import.feature which documents a reduced coverage after SonarQube update.

jump from 88.9 to 83 in sonar 5.x. enable once 5.x LTS is here

guwirth · 2017-09-23T10:10:05Z

@tomassdepakin thsnks for the fix.

In general looks good. Coverage is increased after your fix. It's always hard to say if coverage values are correct or not because every tool is doing/counting it al little bit different. Can you compare the numbers of our integration tests with the values shown in Bullseye please. Are they better/same/worse?

guwirth · 2017-09-23T10:12:03Z

@tomassdepakin files you have to adapt the values if they are right are:

Bertk · 2017-09-25T10:59:42Z

@tomassdepakin The integration tests still fail and you have to adapt the values within the scenarios for the Sonarqube 5.6 tests to pass the checks.

https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/integration-tests/features/importing_coverage.feature
https://github.com/SonarOpenCommunity/sonar-cxx/blob/master/integration-tests/features/smoketest.feature

Feature: Importing coverage data
  @SqApi56
  Scenario: Importing coverage reports
      Assertion Failed: 
        overall_line_coverage is actually 28.2 [expected: 20.0]
        line_coverage is actually 17.6 [expected: 12.5]
        it_line_coverage is actually 36.4 [expected: 26.3]
        coverage is actually 23.8 [expected: 20.0]
        overall_coverage is actually 34.0 [expected: 28.6]
        it_coverage is actually 40.6 [expected: 34.5]
        
  @SqApi56
  Scenario: Importing coverage reports zeroing coverage for untouched files 
      Assertion Failed: 
        overall_line_coverage is actually 20.0 [expected: 13.7]
        line_coverage is actually 7.3 [expected: 5.0]
        it_line_coverage is actually 19.0 [expected: 12.8]
        coverage is actually 11.1 [expected: 9.1]
        overall_coverage is actually 26.1 [expected: 21.5]
        it_coverage is actually 25.0 [expected: 20.4]
        

Feature: Smoketest
  @SqApi56
  Scenario: Smoketest
      Assertion Failed: 
        overall_coverage is actually 84.0 [expected: 81.8]
        it_coverage is actually 84.0 [expected: 81.8]
        coverage is actually 84.0 [expected: 81.8]

tomassdepakin · 2017-09-25T12:11:44Z

Yep, I want to make more tests on coverage computing, and also with SQ5.6

tomassdepakin · 2017-09-26T13:15:52Z

Bullseye counts "for-loop", "try", "catch", "switch" as 1 decision, but we counted each of them as 2.
Fixed to treat such statements as 1 condition.
Simplified condition\decision probes to be treated in same way. In the end SQ has only conditions.
Fixed uncovered functions to be laso shown in UI (previously weren't, see first post image L114).
After applying all fixes, uncovered conditions count in SQ UI is same as in bullseye reports.

Same with try, catch, switch blocks
Integration tests still to be fixed.

tomassdepakin · 2017-09-26T15:04:11Z

Checked with SQ 5.6 - UI and measures are same as in SQ 6.3
6.3:

5.6:

Overall coverage measures are very close to those in bullseye reports. In our project there is 1-5% difference per file.
Only difference left is line coverage. The more uncovered conditions - the more difference in overall coverage, because we mark them as "lineHits = 0" and SQ says to cover them not only as condition, but also as line.

Bertk · 2017-09-26T18:34:58Z

The coverage values for SQ6.2 and newer have now to be adopted as well.

  @SqApi62
  Scenario: Importing coverage reports
      	line_coverage is actually 25.0 [expected: 17.5]
      	coverage is actually 31.0 [expected: 25.9]
  @SqApi62
  Scenario: Smoketest 
      	coverage is actually 53.8 [expected: 50.0]
      	line_coverage is actually 54.8 [expected: 50.0]

guwirth · 2017-09-26T21:05:28Z

@tomassdepakin please rebase

Refactors CoverageMeasure to not be divided to line and condition type

tomassdepakin · 2017-09-27T12:10:33Z

@guwirth done

guwirth · 2017-09-27T15:58:52Z

@Bertk is final version also ok for you?

guwirth · 2017-09-27T15:59:59Z

@tomassdepakin looks good to me. Thanks for providing this PR.

Bertk

👍 Great job. Thanks for fixing the coverage indication in SQ source code UI.

tomassdepakin · 2017-09-28T07:43:08Z

Thank YOU guys for developing this plugin) My pleasure to participate.

Bertk suggested changes Sep 23, 2017

View reviewed changes

guwirth added the bug label Sep 23, 2017

guwirth added this to the 0.9.8 milestone Sep 23, 2017

guwirth assigned tomassdepakin Sep 23, 2017

tomassdepakin force-pushed the bugfix/bullseye_coverage branch from 553906d to 504fef7 Compare September 25, 2017 10:10

Bertk approved these changes Sep 25, 2017

View reviewed changes

Bertk mentioned this pull request Sep 26, 2017

fix feature file quality flaws #1238

Merged

Pavel Pavlov added 5 commits September 27, 2017 09:34

Fixes bullseye conditions to be shown in SQ UI

b45d7ee

Refactors CoverageMeasure to not be divided to line and condition type

Fixes non-covered functions to be shown in SQ UI

9fa40e4

Fixes bullseye conditions count, uncovered functions in UI

6ab1c54

Fixes integration tests related to bullseye coverage improvements

b71f9f7

More fixes to integration tests

37c9fd3

tomassdepakin force-pushed the bugfix/bullseye_coverage branch from f7b1001 to 37c9fd3 Compare September 27, 2017 06:59

guwirth self-requested a review September 27, 2017 15:57

guwirth approved these changes Sep 27, 2017

View reviewed changes

guwirth requested a review from Bertk September 27, 2017 15:59

Bertk approved these changes Sep 27, 2017

View reviewed changes

guwirth merged commit 0f1ccc1 into SonarOpenCommunity:master Sep 28, 2017

tomassdepakin deleted the bugfix/bullseye_coverage branch September 28, 2017 10:12

guwirth mentioned this pull request Oct 29, 2017

RFF V0.9.8 with SQ6.x #1196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes bullseye conditions to be shown in SQ UI #1237

Fixes bullseye conditions to be shown in SQ UI #1237

tomassdepakin commented Sep 22, 2017 •

edited

Loading

Bertk commented Sep 23, 2017

Bertk Sep 23, 2017

tomassdepakin Sep 25, 2017

Bertk Sep 23, 2017

tomassdepakin Sep 25, 2017

Bertk commented Sep 23, 2017 •

edited

Loading

guwirth commented Sep 23, 2017

guwirth commented Sep 23, 2017

Bertk commented Sep 25, 2017

tomassdepakin commented Sep 25, 2017

tomassdepakin commented Sep 26, 2017 •

edited

Loading

tomassdepakin commented Sep 26, 2017

Bertk commented Sep 26, 2017

guwirth commented Sep 26, 2017

tomassdepakin commented Sep 27, 2017

guwirth commented Sep 27, 2017

guwirth commented Sep 27, 2017

Bertk left a comment

tomassdepakin commented Sep 28, 2017

Fixes bullseye conditions to be shown in SQ UI #1237

Fixes bullseye conditions to be shown in SQ UI #1237

Conversation

tomassdepakin commented Sep 22, 2017 • edited Loading

Bertk commented Sep 23, 2017

Bertk Sep 23, 2017

Choose a reason for hiding this comment

tomassdepakin Sep 25, 2017

Choose a reason for hiding this comment

Bertk Sep 23, 2017

Choose a reason for hiding this comment

tomassdepakin Sep 25, 2017

Choose a reason for hiding this comment

Bertk commented Sep 23, 2017 • edited Loading

guwirth commented Sep 23, 2017

guwirth commented Sep 23, 2017

Bertk commented Sep 25, 2017

tomassdepakin commented Sep 25, 2017

tomassdepakin commented Sep 26, 2017 • edited Loading

tomassdepakin commented Sep 26, 2017

Bertk commented Sep 26, 2017

guwirth commented Sep 26, 2017

tomassdepakin commented Sep 27, 2017

guwirth commented Sep 27, 2017

guwirth commented Sep 27, 2017

Bertk left a comment

Choose a reason for hiding this comment

tomassdepakin commented Sep 28, 2017

tomassdepakin commented Sep 22, 2017 •

edited

Loading

Bertk commented Sep 23, 2017 •

edited

Loading

tomassdepakin commented Sep 26, 2017 •

edited

Loading