Skip to content

RC-350 Unify ruling expectation file locations - #6103

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

RC-350 Unify ruling expectation file locations#6103
romainbrenguier wants to merge 1 commit into
masterfrom
romain/normalize-ruling-expectations

Conversation

@romainbrenguier

Copy link
Copy Markdown
Contributor

Summary

  • Move ruling expectation files from its/ruling/src/test/resources/<project>/ to its/ruling/src/test/resources/expected/java/<project>/
  • Update JavaRulingTest to reference the new path

Test plan

  • Ruling tests pass on CI with the new file locations

🤖 Generated with Claude Code

Move ruling expectation files from
  its/ruling/src/test/resources/<project>/
to
  its/ruling/src/test/resources/expected/java/<project>/

This normalizes the expectation file locations to be consistent
across all analyzers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

RC-350


private static void prepareDumpOldFolder() throws Exception {
Path allRulesFolder = Paths.get("src/test/resources");
Path allRulesFolder = Paths.get("src/test/resources/expected/java");

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: CI ruling-root inputs still point at the old expectation root

ruling-update-and-notify copies the merged actual_* artifacts straight into ruling-root (cp -rv "$TEMP_DIR"/* "${ruling-root}/" in SonarSource/core-languages-tooling-public/ruling-update-and-notify/action.yml:302), and the artifacts are laid out as <project>/java-SXXXX.json. With ruling-root: 'its/ruling/src/test/resources' unchanged, the auto-update job will recreate the pre-PR its/ruling/src/test/resources/<project>/ tree instead of updating expected/java/<project>/, so the generated update PR never fixes the ruling failure that JavaRulingTest reads from src/test/resources/expected/java. Likewise ruling-diff-comment parses each changed path with parse_ruling_relative_path, which requires exactly <project>/<repo>-<rule>.json after stripping ruling-root; the new paths yield expected/java/<project>/java-SXXXX.json (4 parts), so every ruling file is dropped with "Skipping unrecognized ruling path" and no diff comment is ever posted again. Point both ruling-root inputs at its/ruling/src/test/resources/expected/java.

build.yml: point the auto-update action at the new expectation root (apply the same change to ruling-diff-comment.yml:56):

ruling-root: 'its/ruling/src/test/resources/expected/java'
  • Apply fix

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

@gitar-bot

gitar-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown
Code Review ⚠️ Changes requested 0 resolved / 1 findings

Refactors ruling expectation file locations to its/ruling/src/test/resources/expected/java/<project>/ but CI inputs still reference the old root. The ruling-root parameters in the auto-update and diff-comment jobs must be updated to its/ruling/src/test/resources/expected/java to prevent the ruling-update workflow from recreating the pre-PR directory structure and to ensure diff comments are generated correctly.

⚠️ Bug: CI ruling-root inputs still point at the old expectation root

📄 its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:160

ruling-update-and-notify copies the merged actual_* artifacts straight into ruling-root (cp -rv "$TEMP_DIR"/* "${ruling-root}/" in SonarSource/core-languages-tooling-public/ruling-update-and-notify/action.yml:302), and the artifacts are laid out as <project>/java-SXXXX.json. With ruling-root: 'its/ruling/src/test/resources' unchanged, the auto-update job will recreate the pre-PR its/ruling/src/test/resources/<project>/ tree instead of updating expected/java/<project>/, so the generated update PR never fixes the ruling failure that JavaRulingTest reads from src/test/resources/expected/java. Likewise ruling-diff-comment parses each changed path with parse_ruling_relative_path, which requires exactly <project>/<repo>-<rule>.json after stripping ruling-root; the new paths yield expected/java/<project>/java-SXXXX.json (4 parts), so every ruling file is dropped with "Skipping unrecognized ruling path" and no diff comment is ever posted again. Point both ruling-root inputs at its/ruling/src/test/resources/expected/java.

build.yml: point the auto-update action at the new expectation root (apply the same change to ruling-diff-comment.yml:56)
ruling-root: 'its/ruling/src/test/resources/expected/java'
🤖 Prompt for agents
Code Review: Refactors ruling expectation file locations to `its/ruling/src/test/resources/expected/java/<project>/` but CI inputs still reference the old root. The `ruling-root` parameters in the auto-update and diff-comment jobs must be updated to `its/ruling/src/test/resources/expected/java` to prevent the ruling-update workflow from recreating the pre-PR directory structure and to ensure diff comments are generated correctly.

1. ⚠️ Bug: CI ruling-root inputs still point at the old expectation root
   Files: its/ruling/src/test/java/org/sonar/java/it/JavaRulingTest.java:160

   `ruling-update-and-notify` copies the merged `actual_*` artifacts straight into `ruling-root` (`cp -rv "$TEMP_DIR"/* "${ruling-root}/"` in SonarSource/core-languages-tooling-public/ruling-update-and-notify/action.yml:302), and the artifacts are laid out as `<project>/java-SXXXX.json`. With `ruling-root: 'its/ruling/src/test/resources'` unchanged, the auto-update job will recreate the pre-PR `its/ruling/src/test/resources/<project>/` tree instead of updating `expected/java/<project>/`, so the generated update PR never fixes the ruling failure that `JavaRulingTest` reads from `src/test/resources/expected/java`. Likewise `ruling-diff-comment` parses each changed path with `parse_ruling_relative_path`, which requires exactly `<project>/<repo>-<rule>.json` after stripping `ruling-root`; the new paths yield `expected/java/<project>/java-SXXXX.json` (4 parts), so every ruling file is dropped with "Skipping unrecognized ruling path" and no diff comment is ever posted again. Point both `ruling-root` inputs at `its/ruling/src/test/resources/expected/java`.

   Fix (build.yml: point the auto-update action at the new expectation root (apply the same change to ruling-diff-comment.yml:56)):
   ruling-root: 'its/ruling/src/test/resources/expected/java'

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

@sonarqube-next

sonarqube-next Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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