Skip to content

SONARJAVA-6641 Fix ruling diff not working for some projects#5809

Draft
romainbrenguier wants to merge 4 commits into
masterfrom
romain/test-ruling-sync
Draft

SONARJAVA-6641 Fix ruling diff not working for some projects#5809
romainbrenguier wants to merge 4 commits into
masterfrom
romain/test-ruling-sync

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

This is to test the workflows.

Part of


Summary by Gitar

  • CI Configuration:
    • Updated .github/workflows/ruling-diff-comment.yml to use submodules: recursive and create a symlink for sonar-server to fix pathing issues.
    • Updated the workflow to use the romain/read-sources-from-disk version of ruling-diff-comment.
  • Test Configuration:
    • Added missing line 60 expectations across multiple JSON ruling files in its/ruling/src/test/resources/.

This will update automatically on new commits.

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Updated expected ruling diff for testing SONARJAVA-6641 Updated expected ruling diff for testing Jul 17, 2026
@hashicorp-vault-sonar-prod

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

Copy link
Copy Markdown
Contributor

SONARJAVA-6641

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 10 rule files: 0 issues removed, 10 issues added.

S112 (java) on commons-beanutils - 0 issues removed, 1 issues added

Added src/main/java/org/apache/commons/beanutils2/BeanComparator.java (line 62)

        57 |      * <strong>Note</strong> that this is intended to be used
        58 |      * only in bean-centric environments.
        59 |      * </p><p>
        60 |      * Until {@link #setProperty} is called with a non-null value.
        61 |      * this comparator will compare the Objects only.
>>>     62 |      * </p>
        63 |      */
        64 |     public BeanComparator() {
        65 |         this( null );
        66 |     }
        67 | 
S139 (java) on eclipse-jetty - 0 issues removed, 1 issues added

Added jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java (line 61)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java)
S108 (java) on eclipse-jetty-similar-to-main - 0 issues removed, 1 issues added

Added jetty-jmx/src/test/java/org/eclipse/jetty/jmx/MBeanContainerLifeCycleTest.java (line 60)

(source file not found at this revision: jetty-jmx/src/test/java/org/eclipse/jetty/jmx/MBeanContainerLifeCycleTest.java)
S121 (java) on eclipse-jetty-similar-to-main-small - 0 issues removed, 1 issues added

Added jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java (line 60)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java)
S103 (java) on guava - 0 issues removed, 1 issues added

Added src/com/google/thirdparty/publicsuffix/PublicSuffixPatterns.java (line 60)

        55 |   /**
        56 |    * The elements in this map would pass the UNDER test, but are known not to
        57 |    * be public suffixes and are thus excluded from consideration. Since it
        58 |    * refers to elements in UNDER of the same type, the type is actually not
        59 |    * important here. The map is simply used for consistency reasons.
>>>     60 |    */
        61 |   public static final ImmutableMap<String, PublicSuffixType> EXCLUDED =
        62 |       TrieParser.parseTrie("kc.www?pj.&a&mahokoy.ytic?yogan.ytic??ebok.ytic?i&adnes.ytic?kasawak.ytic??oroppas.ytic?uhsuykatik.ytic??zm.atadelet??");
        63 | }
S105 (java) on jboss-ejb3-tutorial - 0 issues removed, 1 issues added

Added cachedentity/src/org/jboss/tutorial/cachedentity/client/CachedEntityRun.java (line 60)

        55 |       prop2.put("java.naming.factory.url.pkgs", "org.jboss.naming:org.jnp.interfaces");
        56 |       prop2.put("java.naming.provider.url", "jnp://" + args[1]);
        57 | 
        58 |       System.out.println("Saving customer to node1 = " + args[0]);
        59 |       InitialContext ctx1 = new InitialContext(prop1);
>>>     60 | 
        61 |       EntityTest tester1 = (EntityTest)ctx1.lookup("EntityTestBean/remote");
        62 |       Customer customer = tester1.createCustomer();
        63 |       customer = tester1.findByCustomerId(customer.getId());
        64 | 
        65 |       System.out.println("Looking for customer on node2 = " + args[1] + " (should be available in cache)");
S106 (java) on mall - 0 issues removed, 1 issues added

Added mall-mbg/src/main/java/com/macro/mall/Generator.java (line 60)

(requested line 60 not present, showing closest line 38)
        33 |         //输出警告信息
        34 |         for (String warning : warnings) {
        35 |             System.out.println(warning);
        36 |         }
        37 |     }
>>>     38 | }
S1134 (java) on regex-examples - 0 issues removed, 1 issues added

Added src/main/java/org/regex/examples/RegexDatabase4.java (line 60)

        55 |     Pattern.compile(".*<stream:stream[^>]*id=['|\"]([^'|^\"]+)['|\"][^>]*>.*", Pattern.DOTALL),
        56 |     // https://github.com/harlanji/soashable/blob/ad383f88ff7fc8dc1c407c8c0eb49a08c877f809/jhb/src/main/java/org/jabber/JabberHTTPBind/Session.java#L333
        57 |     Pattern.compile(".*<stream:stream[^>]*id=['|\"]([^'|^\"]+)['|\"][^>]*>.*(<stream.*)$", Pattern.DOTALL),
        58 |     // https://github.com/harlanji/soashable/blob/ad383f88ff7fc8dc1c407c8c0eb49a08c877f809/jhb/src/main/java/org/jabber/JabberHTTPBind/Session.java#L337
        59 |     Pattern.compile(".*<stream:stream[^>]*version=['|\"]1.0['|\"][^>]*>.*", Pattern.DOTALL),
>>>     60 |     // https://github.com/bobmcwhirter/jboss-rails/blob/c0809da9a4ee8afad56b37547fafce25e2bc1232/src/main/java/org/jboss/rails/core/deployers/RailsVersionDeployer.java#L67
        61 |     Pattern.compile("^\\s*MAJOR\\s*=\\s*([0-9]+)\\s*$"),
        62 |     // https://github.com/bobmcwhirter/jboss-rails/blob/c0809da9a4ee8afad56b37547fafce25e2bc1232/src/main/java/org/jboss/rails/core/deployers/RailsVersionDeployer.java#L68
        63 |     Pattern.compile("^\\s*MINOR\\s*=\\s*([0-9]+)\\s*$"),
        64 |     // https://github.com/bobmcwhirter/jboss-rails/blob/c0809da9a4ee8afad56b37547fafce25e2bc1232/src/main/java/org/jboss/rails/core/deployers/RailsVersionDeployer.java#L69
        65 |     Pattern.compile("^\\s*TINY\\s*=\\s*([0-9]+)\\s*$"),
S5838 (java) on sonar-server - 0 issues removed, 1 issues added

Added src/test/java/org/sonar/server/app/TomcatContextsTest.java (line 60)

(source file not found at this revision: src/test/java/org/sonar/server/app/TomcatContextsTest.java)
S8924 (java) on sonar-server - 0 issues removed, 1 issues added

Added src/test/java/org/sonar/ce/log/CeLoggingTest.java (line 60)

(source file not found at this revision: src/test/java/org/sonar/ce/log/CeLoggingTest.java)

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5810

Please review and merge it into your branch.

@romainbrenguier
romainbrenguier force-pushed the romain/test-ruling-sync branch from 590f014 to 98ddeba Compare July 17, 2026 12:46
@romainbrenguier romainbrenguier changed the title SONARJAVA-6641 Updated expected ruling diff for testing SONARJAVA-6641 Fix ruling diff not working for some projects Jul 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5810

Please review and merge it into your branch.

…rver symlink

- Change submodules checkout from 'true' to 'recursive' to include nested submodules
  (eclipse-jetty, eclipse-jetty-similar-to-main, eclipse-jetty-similar-to-main-small)
- Add step to create symlink for sonar-server sources at expected location
  (its/sources/sonar-server -> sonarqube-6.5/server/sonar-server)

This fixes the '(source file not found at this revision)' errors in ruling diff comments.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5810

Please review and merge it into your branch.

Point to core-languages-tooling-public@romain/read-sources-from-disk
which reads source files from disk instead of git show, fixing support
for nested submodules (eclipse-jetty) and non-standard layouts (sonar-server).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqube-next

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Ruling needs updating. A fix PR has been created: #5810

Please review and merge it into your branch.

@gitar-bot

gitar-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown
CI failed: Integration tests are failing due to discrepancies in analysis output and invalid pointer configurations, suggesting that recent changes to ruling logic or file-reading mechanisms have invalidated existing test expectations.

Overview

Multiple integration test failures were observed across the its/ruling module. The tests are failing both due to mismatches between generated and expected analysis reports and internal errors in the LITS (Language Integrated Testing Suite) processing, likely caused by the PR's changes to file reading and ruling synchronization.

Failures

Ruling Diff Assertion Failure (confidence: high)

  • Type: test
  • Affected jobs: 87833942342, 87902822579
  • Related to change: yes
  • Root cause: The JavaRulingTest identified discrepancies between actual analysis output and the stored expected JSON files (e.g., Issues differences: 1). This is a direct result of changes in how ruling diffs are generated or how source files are read.
  • Suggested fix: Inspect the generated diffs in its/ruling/target/sonar-server_differences. If the analysis changes are intentional and correct, regenerate the test resources using the updated outputs.

Invalid File Pointer Error in LITS (confidence: high)

  • Type: test
  • Affected jobs: 87902822579
  • Related to change: yes
  • Root cause: The LITS process failed with 60 is not a valid line for pointer. File ... has 39 line(s). This indicates that the new logic for locating issues or reading files is producing file pointers that exceed the actual file length in the test environment.
  • Suggested fix: Review the com.sonarsource.lits.DumpPhase logic and ensure that file pointer offsets are correctly calculated and validated against the actual file line counts.

Summary

  • Change-related failures: 2 (Test assertion failures and invalid file pointer indexing).
  • Infrastructure/flaky failures: 0 (Develocity connectivity timeouts observed, but these are warnings/non-blocking for test outcome).
  • Recommended action: The developer needs to verify the correctness of the new ruling logic and synchronize the test resources. The specific LITS failure suggests a regression in how files are indexed or parsed within the new diff-based testing mechanism.
Code Review ✅ Approved

Updates ruling diff CI workflows to use recursive submodules and correct symlink paths for the sonar-server subproject. Updated expected test resources align with current output requirements.

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.

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

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

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