Skip to content

SONARJAVA-6586 Demonstrate ruling test report upload to GitHub#5757

Draft
romainbrenguier wants to merge 1 commit into
masterfrom
romain/ruling-report
Draft

SONARJAVA-6586 Demonstrate ruling test report upload to GitHub#5757
romainbrenguier wants to merge 1 commit into
masterfrom
romain/ruling-report

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR demonstrates the existing ruling test report upload functionality in sonar-java. The infrastructure for uploading ruling test reports to GitHub as artifacts when tests fail is already implemented and working.

What This PR Does

  1. Documents the existing upload mechanism in RULING_REPORT_UPLOAD.md
  2. Tests the functionality by intentionally modifying S1118 expected results to trigger a test failure

Infrastructure Already in Place

The repository already has:

✅ Custom GitHub Action (.github/actions/upload-actual/action.yml) that:

  • Uploads raw JSON results from ruling tests
  • Generates HTML diff reports using diff2html-cli
  • Retains artifacts for 7 days

✅ Workflow integration in .github/workflows/build.yml:

  • Automatically runs on ruling test failures
  • Uploads for multiple configurations (Linux/Windows, with/without SQ project)

✅ Report generation via sonar-lits-plugin:

  • JSON files per rule per project showing detected issues
  • Differences report comparing expected vs actual results

Expected CI Behavior

When the ruling tests run in CI, they will:

  1. Fail on the guava project test (S1118 rule)
  2. 📦 Upload artifacts:
    • actual_<runner>_<profile> - Raw JSON files with detected issues
    • diff_<runner>_<profile>.html - Visual HTML diff showing changes

Test Change

Modified its/ruling/src/test/resources/guava/java-S1118.json:

  • Removed one expected issue from Types.java:618
  • This will cause the analyzer to detect an issue that's not in the baseline
  • Demonstrates the diff reporting functionality

How to Verify

  1. Wait for CI to run the ruling-qa job
  2. The test will fail (as intended)
  3. Navigate to the failed job in GitHub Actions
  4. Download the artifacts to see:
    • The actual detected issues (JSON format)
    • The visual diff report (HTML format)

Comparison with sonar-python-enterprise

This implementation is identical to sonar-python-enterprise:

  • Same sonar-lits-plugin for report generation
  • Same artifact upload mechanism
  • Same HTML diff visualization
  • Same 7-day retention period

Note

This is a demonstration PR. The test modification should not be merged as it intentionally breaks the ruling test.

🤖 Generated with Claude Code


Summary by Gitar

  • New automated PR commenting:
    • Added ruling-diff-comment GitHub Action and workflow to automatically post/update issue deltas on PRs when ruling JSON files change.
    • Implemented Python logic in ruling_diff.py to calculate and format added/removed issues by comparing base and head commits, including source code snippets.
  • Documentation:
    • Added RULING_REPORT_UPLOAD.md providing a comprehensive guide on ruling test infrastructure, report generation, and the new PR comment functionality.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Demonstrate ruling test report upload to GitHub SONARJAVA-6586 Demonstrate ruling test report upload to GitHub Jul 7, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6586

Comment on lines 8 to 13
"com.google.guava:guava:src/com/google/common/collect/ImmutableMultimap.java": [
343
],
"com.google.guava:guava:src/com/google/common/reflect/Types.java": [
618
],
"com.google.guava:guava:src/com/google/thirdparty/publicsuffix/TrieParser.java": [
30
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Intentional ruling-test breakage must not be merged

Commit b675e7c removes the expected S1118 issue at com.google.guava:guava:src/com/google/common/reflect/Types.java:618 from the baseline JSON. As the PR description itself states, this is a deliberate demonstration change that will cause the guava ruling test (JavaRulingTest) to fail in CI, since the analyzer still detects an issue at Types.java:618 that is no longer present in the baseline.

While this is intentional for demonstrating the artifact-upload workflow, the change is not safe to merge into master: it will leave the ruling QA job permanently red. Before merging, this baseline entry must be restored:

"com.google.guava:guava:src/com/google/common/reflect/Types.java": [
618
],

Recommendation: keep only the documentation commit (bce24c6) on the merge branch and drop/revert the baseline modification, or close this PR as a demonstration-only branch.

Restore the removed expected issue so the guava ruling test passes.:

"com.google.guava:guava:src/com/google/common/collect/ImmutableMultimap.java": [
343
],
"com.google.guava:guava:src/com/google/common/reflect/Types.java": [
618
],
"com.google.guava:guava:src/com/google/thirdparty/publicsuffix/TrieParser.java": [
30
]
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This PR is blocked due to unresolved code review findings.

Comment gitar unblock to override this block and allow merging.

Configure merge blocking · Maintainers can dismiss this review.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 1 rule file(s): 1 issue(s) removed, 3 issue(s) added.

S1118 (java) on guava - 1 issues removed, 3 issues added

Removed src/com/google/common/reflect/Types.java (line 618)

      613 |    * Moreover, we don't want to reconstruct a native type variable <A> using our implementation
      614 |    * unless some of its bounds have changed in resolution. This avoids creating unequal TypeVariable
      615 |    * implementation unnecessarily. When the bounds do change, however, it's fine for the synthetic
      616 |    * TypeVariable to be unequal to any native TypeVariable anyway.
      617 |    */
>>>   618 |   static final class NativeTypeVariableEquals<X> {
      619 |     static final boolean NATIVE_TYPE_VARIABLE_ONLY =
      620 |         !NativeTypeVariableEquals.class.getTypeParameters()[0].equals(
      621 |             newArtificialTypeVariable(NativeTypeVariableEquals.class, "X"));
      622 |   }
      623 | 

Added src/com/google/common/collect/ImmutableMultimap.java (line 512)

      507 |     return (ImmutableMap) map;
      508 |   }
      509 | 
      510 |   @Override
      511 |   Map<K, Collection<V>> createAsMap() {
>>>   512 |     throw new AssertionError("should never be called");
      513 |   }
      514 | 
      515 |   /**
      516 |    * Returns an immutable collection of all key-value pairs in the multimap.
      517 |    */

Added src/com/google/common/reflect/TypeToken.java (line 156)

      151 |   /** Returns an instance of type token that wraps {@code type}. */
      152 |   public static <T> TypeToken<T> of(Class<T> type) {
      153 |     return new SimpleTypeToken<T>(type);
      154 |   }
      155 | 
>>>   156 |   /** Returns an instance of type token that wraps {@code type}. */
      157 |   public static TypeToken<?> of(Type type) {
      158 |     return new SimpleTypeToken<Object>(type);
      159 |   }
      160 | 
      161 |   /**

Added src/com/google/common/reflect/TypeToken.java (line 892)

      887 |   }
      888 | 
      889 |   /**
      890 |    * Return true if any of the following conditions is met: <ul>
      891 |    * <li>'this' and {@code formalType} are equal
>>>   892 |    * <li>{@code formalType} is {@code <? extends Foo>} and 'this' is a subtype of {@code Foo}
      893 |    * <li>{@code formalType} is {@code <? super Foo>} and 'this' is a supertype of {@code Foo}
      894 |    * </ul>
      895 |    */
      896 |   private boolean is(Type formalType) {
      897 |     if (runtimeType.equals(formalType)) {

Implements a GitHub Action workflow that automatically posts detailed
comments on pull requests when ruling test expected results change.

The comment includes:
- Summary of changed rules and issue counts
- Detailed diff showing added/removed issues per file
- Code snippets with context around each issue location
- Proper handling of submodule sources

Implementation details:
- New workflow: .github/workflows/ruling-diff-comment.yml
- Custom action: .github/actions/ruling-diff-comment/
- Automatically triggered on PR changes to ruling test files
- Updates existing comment to avoid spam

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@sonarqube-next

sonarqube-next Bot commented Jul 8, 2026

Copy link
Copy Markdown

@gitar-bot

gitar-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown
CI failed: The CI build failed due to a test regression in `JavaRulingTest.guava`, caused by a mismatch between actual analysis output and the expected ruling test results after your changes.

Overview

The CI pipeline failed during the JavaRulingTest.guava test case, which performs regression testing on analysis outputs. The test correctly identified discrepancies between the current analysis results and the expected baseline resources in the its/ruling module.

Failures

Ruling Test Regression (confidence: high)

  • Type: test
  • Affected jobs: Windows runner jobs
  • Related to change: yes
  • Root cause: The changes introduced in this PR affected the analysis output for rule S1118, resulting in a mismatch in its/ruling/src/test/resources/guava/java-S1118.json. The JavaRulingTest caught this discrepancy and raised an AssertionError.
  • Suggested fix: Review the changes to java-S1118.json. If the changes represent the new, intended behavior of the analyzer, update the test baseline/resources to reflect the new output. If not, investigate why the analyzer is producing different results for this rule.

Summary

  • Change-related failures: 1 (Test failure in JavaRulingTest due to expected/actual resource mismatch).
  • Infrastructure/flaky failures: 0 (Note: observed intermittent Develocity connection warnings, but these did not cause the primary test failure).
  • Recommended action: Verify the output changes in the ruling test resources. Update the checked-in JSON files to match the new analysis results if the change in behavior is intentional.
Code Review 🚫 Blocked 0 resolved / 1 findings

Implements automated PR comments for ruling test diffs and documents the reporting infrastructure. The intentional breakage of the S1118 test in commit b675e7c must be reverted before merging.

🚨 Bug: Intentional ruling-test breakage must not be merged

📄 its/ruling/src/test/resources/guava/java-S1118.json:8-13

Commit b675e7c removes the expected S1118 issue at com.google.guava:guava:src/com/google/common/reflect/Types.java:618 from the baseline JSON. As the PR description itself states, this is a deliberate demonstration change that will cause the guava ruling test (JavaRulingTest) to fail in CI, since the analyzer still detects an issue at Types.java:618 that is no longer present in the baseline.

While this is intentional for demonstrating the artifact-upload workflow, the change is not safe to merge into master: it will leave the ruling QA job permanently red. Before merging, this baseline entry must be restored:

"com.google.guava:guava:src/com/google/common/reflect/Types.java": [
618
],

Recommendation: keep only the documentation commit (bce24c6) on the merge branch and drop/revert the baseline modification, or close this PR as a demonstration-only branch.

Restore the removed expected issue so the guava ruling test passes.
"com.google.guava:guava:src/com/google/common/collect/ImmutableMultimap.java": [
343
],
"com.google.guava:guava:src/com/google/common/reflect/Types.java": [
618
],
"com.google.guava:guava:src/com/google/thirdparty/publicsuffix/TrieParser.java": [
30
]
🤖 Prompt for agents
Code Review: Implements automated PR comments for ruling test diffs and documents the reporting infrastructure. The intentional breakage of the S1118 test in commit b675e7c must be reverted before merging.

1. 🚨 Bug: Intentional ruling-test breakage must not be merged
   Files: its/ruling/src/test/resources/guava/java-S1118.json:8-13

   Commit b675e7c removes the expected S1118 issue at `com.google.guava:guava:src/com/google/common/reflect/Types.java:618` from the baseline JSON. As the PR description itself states, this is a deliberate demonstration change that will cause the `guava` ruling test (JavaRulingTest) to fail in CI, since the analyzer still detects an issue at Types.java:618 that is no longer present in the baseline.
   
   While this is intentional for demonstrating the artifact-upload workflow, the change is not safe to merge into `master`: it will leave the ruling QA job permanently red. Before merging, this baseline entry must be restored:
   
   ```
   "com.google.guava:guava:src/com/google/common/reflect/Types.java": [
   618
   ],
   ```
   
   Recommendation: keep only the documentation commit (bce24c6) on the merge branch and drop/revert the baseline modification, or close this PR as a demonstration-only branch.

   Fix (Restore the removed expected issue so the guava ruling test passes.):
   "com.google.guava:guava:src/com/google/common/collect/ImmutableMultimap.java": [
   343
   ],
   "com.google.guava:guava:src/com/google/common/reflect/Types.java": [
   618
   ],
   "com.google.guava:guava:src/com/google/thirdparty/publicsuffix/TrieParser.java": [
   30
   ]

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change the behavior for this request:

Auto-apply Compact Unblock
gitar auto-apply:on         
gitar display:verbose         
gitar unblock         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant