[go: up one dir, main page]

Page MenuHomePhabricator

SonarCloud not picking up unit test coverage on master and patch branches of extensions and core
Closed, ResolvedPublic

Description

As shown by https://sonarcloud.io/component_measures?id=mediawiki-extensions-CheckUser, SonarCloud is not picking up any code coverage information about the CheckUser extension even though unit test code coverage was previously reported.

This causes all patches on CheckUser to see ❌ coverage (0.0 is less than 20) even when the patches have sufficient unit test to meet the coverage threshold.

MediaWiki core is in the same situation: https://sonarcloud.io/component_measures?metric=Coverage&id=mediawiki-core

The per change / branch reports do have coverage report though.

Event Timeline

Dreamy_Jazz renamed this task from SonarCloud not picking up unit test coverage on CheckUser to SonarCloud not picking up unit test coverage on master branches of CheckUser and core.Sep 28 2023, 8:08 AM
Dreamy_Jazz added a subscriber: pwangai.

@pwangai any thoughts?

For MediaWiki core, the analysis of the branch happens after a change has been merged (postmerge pipeline in Zuul) by triggering https://integration.wikimedia.org/ci/job/mwcore-codehealth-master-non-voting/

If I look for coverage in the console output of the last build:

06:48:42 INFO: Sensor PHPUnit report sensor [php]
06:48:42 INFO: No PHPUnit tests reports provided (see 'sonar.php.tests.reportPath' property)
06:48:42 INFO: No PHPUnit coverage reports provided (see 'sonar.php.coverage.reportPaths' property)

For a change, the triggered job is https://integration.wikimedia.org/ci/job/mwcore-codehealth-patch/ and that has the same output:

08:53:50 INFO: Sensor PHPUnit report sensor [php]
08:53:50 INFO: No PHPUnit tests reports provided (see 'sonar.php.tests.reportPath' property)
08:53:50 INFO: No PHPUnit coverage reports provided (see 'sonar.php.coverage.reportPaths' property)

The container script has:

dockerfiles/sonar-scanner/run-php.sh
if [[ -f /workspace/log/junit.xml ]] && [[ -f /workspace/log/clover.xml ]]; then
    echo "sonar.php.tests.reportPath=/workspace/log/junit.xml" >> sonar-project.properties
    echo "sonar.php.coverage.reportPaths=/workspace/log/clover.xml" >> sonar-project.properties
fi

Then the job does not capture the resulting sonar-project.properties but it cat it:

# Output sonar-project file to assist with debugging:
echo "== sonar-project.properties =="
cat sonar-project.properties

So for the master branch mwcore-codehealth-master-non-voting job:

== sonar-project.properties ==
sonar.sources=resources,languages,includes,src,modules,maintenance
sonar.exclusions=extensions/**/*,mw-config/**/*,node_modules/**/*,services/**/*,vendor/**/*,skins/**/*,logs/**/*,cache/**/*,resources/lib/**/*
sonar.tests=tests

For the changes mwcore-codehealth-patch job has:

== sonar-project.properties ==
sonar.sources=resources,languages,includes,src,modules,maintenance
sonar.exclusions=extensions/**/*,mw-config/**/*,node_modules/**/*,services/**/*,vendor/**/*,skins/**/*,logs/**/*,cache/**/*,resources/lib/**/*
sonar.tests=tests

So in both cases sonar.php.tests.reportPath and sonar.php.coverage.reportPaths are not set. Then for changes the coverage is attached, so that must come from some other settings or mechanism :-\

In both case they run

+ php
-d extension=pcov.so
-d pcov.enabled=1
-d pcov.directory=/workspace/src
-d 'pcov.exclude=@(tests|vendor)@' vendor/bin/phpunit
--exclude-group Dump,Broken,ParserFuzz,Stub
--coverage-clover /workspace/log/clover.xml
--log-junit /workspace/log/junit.xml tests/phpunit/unit

And eventually looking at the console in both cases there is:

Using PHP 7.4.33
Cannot load list of extensions and skins. Output:
Using PHP 7.4.33
PHP Warning:  require_once(/workspace/src/LocalSettings.php): failed to open stream: No such file or directory in /workspace/src/includes/Setup.php on line 212
PHP Fatal error:  require_once(): Failed opening required '/workspace/src/LocalSettings.php' (include_path='/workspace/src/vendor/pear/console_getopt:/workspace/src/vendor/pear/mail:/workspace/src/vendor/pear/mail_mime:/workspace/src/vendor/pear/net_smtp:/workspace/src/vendor/pear/net_socket:/workspace/src/vendor/pear/net_url2:/workspace/src/vendor/pear/pear-core-minimal/src:/workspace/src/vendor/pear/pear_exception:.:/usr/share/php') in /workspace/src/includes/Setup.php on line 212

+ set -e
+ [[ ! -v CODEHEALTH ]]
+ [[ -v CODEHEALTH ]]
+ '[' -f /workspace/log/junit.xml ']'
+ '[' -s /workspace/log/clover.xml ']'
INFO:quibble.commands:<<< Finish: User commands: mediawiki-coverage, in 0.214 s

Which really should fail the build :/

That still does not explains why coverage works for change but does not work on branch :/

TLDR: I don't know what is going on :-]

As extra info, the coverage is failing to be reported correctly for per change when the change is based on the master branch:

One can set the path manually under the project settings (Your Project > Administration > General Settings > Languages > PHP > Tests and Coverage > Files)

Coverage_path.png (1×2 px, 254 KB)

Waiting for the next report to see if it picks up anything

For the moment I have changed CheckUser's quality gate to not give coverage alerts until we figure out the coverage issues

For the moment I have changed CheckUser's quality gate to not give coverage alerts until we figure out the coverage issues

Thanks.

Something has changed in the unit entrypoint which now requires a LocalSettings.php which I find quite odd? @kostajh did some work to ensure true unit tests.

If that is indeed a new requirement, the alternative would be to change the jobs to do a full MediaWiki installation. The jobs use Quibble with --skip-install which skips the MediaWiki installation.

Something has changed in the unit entrypoint which now requires a LocalSettings.php which I find quite odd? @kostajh did some work to ensure true unit tests.

Yes, that was me in r937559 (for T227900). LS.php is needed to get a list of extensions to run tests for, and we don't need more than that.

Depending on your definition of "true unit test", we may or may not be able to have such things in MediaWiki. The unit tests we have make a few assertions that prevent you from using global variables, singletons, etc. But we need to know what extensions (if any) to run tests for.The only way to configure that in MW is with a LocalSettings.php file, so that's what we need in tests, too. This will hopefully change in the future, but we aren't quite ready yet. Even in the shorter term, we may need to figure out a different solution because of T345481.

So yeah, the simplest thing to do would be to provide a LS.php file. We don't need a storage backend, or to install MW.


As an aside, I've always wondered if there's any value in running code coverage via SonarCloud in addition to the bespoke CI job. The coverage generated by SonarCloud has several shortcomings such as:

  • It's based on unit tests, and ignores integration tests
  • It's not published on doc.wikimedia.org
  • It doesn't work well with stacked patches

I wonder if it would actually be preferable to identify what are the issues with our coverage job (the non-SonarCloud one; i.e., why the SonarCloud one exists) and fix those instead.

I wonder if it would actually be preferable to identify what are the issues with our coverage job (the non-SonarCloud one; i.e., why the SonarCloud one exists) and fix those instead.

From what I can tell, the test coverage from jest, qunit and selenium are not reported on doc.wikimedia.org.

I wonder if it would actually be preferable to identify what are the issues with our coverage job (the non-SonarCloud one; i.e., why the SonarCloud one exists) and fix those instead.

From what I can tell, the test coverage from jest, qunit and selenium are not reported on doc.wikimedia.org.

At the time, we (Code Health Group) were interested in Sonar for its ability to scan repos for various code issues; aggregating code coverage was part of the bigger picture of code health. One nice thing about Sonar is that you can see how the issues it finds evolve over time, so e.g. you can look at whether code coverage has decreased over time, or code issues have increased in number, etc.

As an aside, I've always wondered if there's any value in running code coverage via SonarCloud in addition to the bespoke CI job. The coverage generated by SonarCloud has several shortcomings such as:

  • It's based on unit tests, and ignores integration tests
  • It's not published on doc.wikimedia.org
  • It doesn't work well with stacked patches

It's important to note that the primary goal is to eventually achieve comprehensive coverage from all running tests. Some teams diligently monitor their coverage numbers and patterns as they add more functionality to their codebase. This allows them to assess whether they are falling behind in writing tests and may provide the incentive to improve or maintain their coverage numbers.

The projects that benefit the most from coverage reports are typically newer projects. Due to their smaller codebase, they tend to have higher coverage numbers, and most strive to maintain and improve these figures. It's essential for teams to be aware of the percentage of code covered by test cases, even if it's currently low. This awareness can serve as a catalyst for occasional efforts to enhance test coverage.

Change 983476 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] jjb: Install MediaWiki in codehealth coverage jobs

https://gerrit.wikimedia.org/r/983476

kostajh renamed this task from SonarCloud not picking up unit test coverage on master branches of CheckUser and core to SonarCloud not picking up unit test coverage on master and patch branches of CheckUser and core.Dec 15 2023, 8:15 PM
kostajh renamed this task from SonarCloud not picking up unit test coverage on master and patch branches of CheckUser and core to SonarCloud not picking up unit test coverage on master and patch branches of extensions and core.
kostajh claimed this task.
kostajh triaged this task as High priority.
kostajh added a project: Code-Health.

I wonder if it would actually be preferable to identify what are the issues with our coverage job (the non-SonarCloud one; i.e., why the SonarCloud one exists) and fix those instead.

From what I can tell, the test coverage from jest, qunit and selenium are not reported on doc.wikimedia.org.

At the time, we (Code Health Group) were interested in Sonar for its ability to scan repos for various code issues; aggregating code coverage was part of the bigger picture of code health. One nice thing about Sonar is that you can see how the issues it finds evolve over time, so e.g. you can look at whether code coverage has decreased over time, or code issues have increased in number, etc.

Another thing (that I've missed, which prompted me to make a patch to fix this) is that you can click through the report to see exactly which lines are covered by tests.

Change 983476 merged by jenkins-bot:

[integration/config@master] jjb: Install MediaWiki in codehealth coverage jobs

https://gerrit.wikimedia.org/r/983476