Closed Rots closed 6 years ago
ivangalkin
commented
6 years ago Hi @Rots and thank you for reaching out,
you are right in your assumption: the import is disabled by purpose. I assume, that whoever disabled it tried to suppress warning messages (there is a popular misunderstanding about scanning of multi-module projects, please read #1508 if interested).
I believe, that there is no justification to keep this limitation anymore. @guwirth is that correct? can we remove the check?
ivangalkin
commented
6 years ago Hi All,
I took a closer look at CxxXunitSensor and found out, that it's not able to distinguish, which test-case belongs to which (sub-)module. So the standard algorithm for a sensor doesn't work here:
Accepting the sensor on the project level only is a valid solution in this case. The price of this workaround is a lack of a test results for sub-modules.
The right solution for the root project detection is to check the module definition. The following diff shows the main principle (not for production!). One has to pass ProjectDefinition while creation of the sensor (see CxxUnitTestResultsImportSensor).
I will take care of this issue.
diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java
index e174f0c..9963cd2 100644
--- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java
+++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java
@@ -30,6 +30,7 @@ import javax.xml.stream.XMLStreamException;
import javax.xml.transform.Source;
import javax.xml.transform.TransformerException;
import javax.xml.transform.stream.StreamSource;
+
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.measures.CoreMetrics;
@@ -86,7 +87,7 @@ public class CxxXunitSensor extends CxxReportSensor {
@Override
public void execute(SensorContext context) {
String moduleKey = context.config().get("sonar.moduleKey").orElse(null);
- if (moduleKey != null) {
+ if (((DefaultInputModule) context.module()).definition().getParent() != null) {
LOG.debug("Runs unit test import sensor only at top level project skip : Module Key = '{}'", moduleKey);
return;
}
guwirth
commented
6 years ago I believe, that there is no justification to keep this limitation anymore. @guwirth is that correct? can we remove the check?
@ivangalkin I'm sorry I can't tell you that. Think we have the problem that this feature was growing over the time and SQ versions. I don't use the multi-module feature I know only that we had a lot of trouble with it over the time.
Think makes sense to fix it / verify it again with SQ 6.7 LTS. I'm also wondering if the wiki is still valid? https://github.com/SonarOpenCommunity/sonar-cxx/wiki/Path-and-path-separator-issues
https://github.com/SonarOpenCommunity/sonar-cxx/blob/d4fc380e8acf6d0357b56d7860eb1d5e7681abcf/cxx-sensors/src/main/java/org/sonar/cxx/sensors/tests/xunit/CxxXunitSensor.java#L88
The
sonar.moduleKeyseems to be set in maven executions for even root projects, so the xUnit reports are never parsed:To test I created a simple
sonar-cxx-plugin/integration-tests/testdata/googletest_project/pom.xml:and ran
mvn sonar:sonar -Dsonar.cxx.xunit.reportPath=gtest.xml -Dsonar.sources=src -Dsonar.scm.disabled=true -Dsonar.tests=tests/unittests -Xand in the log above you can see that the xUnit wasn't run with the log message that this has a moduleKey assigned: