Repository navigation
SONARJAVA-7136 Add opt-in source-only Java analysis comparison - #6307
matthew-elliott-sonarsource wants to merge 8 commits into
Conversation
hashicorp-vault-sonar-prod
Bot
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| Previous results are preserved, including across Maven clean builds. Maven and | ||
| IntelliJ use the same location, and the selected directory is printed when the | ||
| class starts. Keep saved run directories in Git so results can be reviewed |
There was a problem hiding this comment.
💡 Quality: README still says results dir is printed when the class starts
This commit removes the @BeforeAll createResultsDirectory() hook. The run directory is now created and printed inside compare_current_and_candidate_source_only_analysis, after the sources are copied and the rules are loaded. The README still says "the selected directory is printed when the class starts". That is wrong now: running only the smoke test prints nothing, and in the comparison test the path is printed partway through the test. Change the sentence to say the directory is printed when the comparison test runs.
Update the README to match where the directory is now created and printed:
Previous results are preserved, including across Maven clean builds. Maven and
IntelliJ use the same location, and the comparison test prints the selected
directory when it creates it. Keep saved run directories in Git so results can be reviewed
alongside the test.
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
72eafcf to
3f58a20
Compare
| boolean complete = files.equals(expectedFiles); | ||
| SemanticReport semantics = SemanticReport.read(semanticReportPath, expectedFiles); | ||
| return new SourceOnlyComparison.Run(label, complete, scanMillis, files, findings, telemetry, | ||
| semantics, | ||
| complete ? null : "Indexed files differ from the intended production Java files. Expected: " + expectedFiles + "; actual: " + files); | ||
| } catch (IOException | RuntimeException e) { | ||
| var stacktrace = new StringWriter(); | ||
| e.printStackTrace(new PrintWriter(stacktrace)); | ||
| return failed(label, TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start), stacktrace.toString()); | ||
| } |
There was a problem hiding this comment.
💡 Quality: Semantic-report failure discards scan results and hides file mismatch
In scan(), SemanticReport.read(semanticReportPath, expectedFiles) runs after complete is computed. Its IOException is caught by the outer catch, which returns failed(...). When the indexed files differ from expectedFiles, the semantic report almost always fails the same coverage check. The run is then recorded as a scanner FAILURE with 0 files and 0 findings, and the error text is a semantic-report stack trace instead of the specific "Indexed files differ..." message. A semantic-report problem after a scan that actually succeeded likewise shows as Scan status | FAILED and drops the files, findings and telemetry from the report. It also makes the semantics() == null branch in SourceOnlyComparison.compare unreachable for real runs. Fix: read the semantic report in its own try block. Keep the scan results, set semantics to null or record the semantic error separately, and keep the indexed-files message when complete is false.
Read semantics separately so a successful scan keeps its data and the file-mismatch message is kept:
boolean complete = files.equals(expectedFiles);
SemanticReport semantics = null;
String semanticError = null;
try {
semantics = SemanticReport.read(semanticReportPath, expectedFiles);
} catch (IOException e) {
semanticError = e.getMessage();
}
String error = !complete ? "Indexed files differ from the intended production Java files. Expected: " + expectedFiles + "; actual: " + files
: semanticError;
return new SourceOnlyComparison.Run(label, complete && semanticError == null, scanMillis, files, findings, telemetry, semantics, error);
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| Run the comparison, its scanner smoke test, and the comparison and semantic-report tests: | ||
|
|
||
| ```sh | ||
| mvn -f its/scanner-integration-tests/pom.xml test \ | ||
| -Dtest=NoCompilationComparisonTest,SourceOnlyComparisonTest,SemanticReportTest \ | ||
| -Dcomparison.project="$HOME/Work/Code/sonar-xml" | ||
| ``` |
There was a problem hiding this comment.
💡 Quality: PR description test counts and run command are stale
The description says "All 9 Maven tests passed" and "the 7 unit tests passed", and its run command lists only NoCompilationComparisonTest,SourceOnlyComparisonTest. It also says the regenerated run-001/report.md is checked in. This commit adds SemanticReportTest (6 tests) and expands SourceOnlyComparisonTest to 10 tests, giving 18 in total. The README command now includes SemanticReportTest, and the new results are in run-002/report.md. Readers following the description will skip the semantic-report tests and look at the wrong report. Fix: update the description's counts, command and report path.
Was this helpful? React with 👍 / 👎
Code Review 👍 Approved with suggestions 1 closed / 4 findings🟡 Medium risk · Adds opt-in source-only scans and semantic-report checks to integration tests. Adds an opt-in harness to compare SonarJava analysis with and without compilation, using the scanner integration tester and generating a Markdown report of findings by rule and location. The comparison runs without project binaries or dependency libraries, and results are saved in numbered run directories. Three minor suggestions before merge: update the README to reflect that the results directory is now printed when the comparison test runs rather than at class start; separate semantic-report error handling from scan-completion logic to preserve scan results and show specific file-mismatch messages instead of stack traces; and refresh the PR description's test counts (now 18 total), run command to include 💡 Quality: README still says results dir is printed when the class starts📄 its/scanner-integration-tests/README.md:47-49 📄 its/scanner-integration-tests/src/test/java/org/sonar/java/it/NoCompilationComparisonTest.java:109-111 This commit removes the Update the README to match where the directory is now created and printed💡 Quality: Semantic-report failure discards scan results and hides file mismatch📄 its/scanner-integration-tests/src/test/java/org/sonar/java/it/NoCompilationComparisonTest.java:190-199 📄 its/scanner-integration-tests/src/test/java/org/sonar/java/it/SourceOnlyComparison.java:84-86 In Read semantics separately so a successful scan keeps its data and the file-mismatch message is kept💡 Quality: PR description test counts and run command are stale📄 its/scanner-integration-tests/README.md:19-25 📄 its/scanner-integration-tests/src/test/java/org/sonar/java/it/SemanticReportTest.java:34-48 📄 its/scanner-integration-tests/src/test/java/org/sonar/java/it/results/run-002/report.md:1 The description says "All 9 Maven tests passed" and "the 7 unit tests passed", and its run command lists only ✅ 1 closed✅ Quality: Committed run logs contain the author's local absolute paths
🤖 Prompt for agentsReview coverage🧪 Functional validation No results 📋 Rules No rules evaluated 🤖 Auto-approval Not enabled · Set up OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This branch has not been deployed
1 participant
Add an opt-in harness for comparing current and future SonarJava analysis without compiling the target project or supplying dependency JARs. It is rebased onto
alban/SemanticReportand now uses the existing OrchestratorMavenBuildrunner.What it does
its/plugin/testsbesideUnitTestsTest.TestUtils.createMavenBuild()throughorchestrator.executeBuild(build), passing the semantic JSON path with.setProperty("sonar.java.internal.semantic.report", path).sonar:sonar, with empty binaries/libraries overrides, the same JDK, Java 21 level, and Sonar way profile. The candidate remains a placeholder until its feature flag exists.comparison.projectand stay out of normal pipeline runs; the helper unit tests remain enabled.results/run-007/report.md; earlier reports are preserved.UnitTestsTest.semantic_report_without_compilationbeside the original example, using its existing Orchestrator and an existing main-source fixture. Also supplies the missing property value intests_with_report_name_suffix.Run the comparison
Build/install the Java plugin and custom-rule example, configure artifact access, and ensure Maven is on PATH (or set
maven.binary):The output property is optional. When supplied, raw JSON is written to
/tmp/report-current.jsonand/tmp/report-candidate.json; otherwise it stays temporary. It selects report output, not the candidate analyzer mode.Small test in the original class
This uses the existing Enterprise lightweight suite and requires valid test-license GitHub access. Its Maven goal is fully qualified to avoid plugin-prefix configuration dependencies. The semantic reporter counts main sources; the original surefire-only fixture can therefore produce an empty report.
Validation
UnitTestsTest: passed, producing JSON for 3 main-source files with 8 identifiers and 0 unknowns. Validation used existing GitHub access in a private temporary Orchestrator config, removed afterward.comparison.project: 18 unit tests passed and the two comparison scanner tests skipped without starting a server or creating a report.target.