RC-350 Unify ruling expectation file locations. - #7922
romainbrenguier wants to merge 1 commit into
Conversation
Move ruling expectation files from its/ruling/src/test/expected/<project>/<language>-<ruleId>.json to its/ruling/src/test/resources/expected/<language>/<project>/<ruleId>.json This makes the layout consistent across analyzers. The Java ruling test (LITS-based) reconstructs the flat per-project directory that LITS expects at build time under target/expected/. The JS/TS ruling pipeline (lits.ts) and the CI ruling bot scripts are updated to read and write the new structure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| Copy actual to expected: | ||
|
|
||
| ```bash | ||
| cp -R target/actual/ src/test/expected/ | ||
| cp -R target/expected/ src/test/resources/expected/ | ||
| ``` | ||
|
|
||
| Review diff: | ||
|
|
||
| ```bash | ||
| diff -rq src/test/expected target/actual | ||
| diff -rq src/test/resources/expected target/expected | ||
| ``` |
There was a problem hiding this comment.
⚠️ Bug: Docs tell you to copy target/expected back over the expectations
RulingTest writes fresh LITS results to target/actual/<project>/<language>-<rule>.json and uses target/expected/<project>/ only as a flat input copy of the committed expectations. So cp -R target/expected/ src/test/resources/expected/ copies the baseline back in the old flat shape (creating expected/<project>/javascript-S100.json next to the new expected/<language>/<project>/S100.json tree) and never picks up the new results, while diff -rq src/test/resources/expected target/expected compares the expectations against a rearranged copy of themselves — it can never show a real ruling change. DEV.md's surviving instruction to copy its/ruling/target/actual/ into its/ruling/src/test/resources/expected/ is wrong for the same reason (flat <language>-<rule>.json names, project-first nesting). Document the actual transposing step (or add a helper/ruling-sync-style script for the Java flow).
SKILL.md: diff the merged baseline against the LITS output, and point at a transposing sync step for updates.:
Review diff:
```bash
diff -rq target/expected target/actual
```
Update the expectations (transposes `<project>/<language>-<ruleId>.json` back to
`<language>/<project>/<ruleId>.json`):
```bash
for f in target/actual/*/*.json; do
project=$(basename "$(dirname "$f")"); name=$(basename "$f")
language=${name%%-*}; rule=${name#*-}
mkdir -p "src/test/resources/expected/$language/$project"
cp "$f" "src/test/resources/expected/$language/$project/$rule"
done
```
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
| private static Path mergeExpectedDir(String projectKey) throws IOException { | ||
| var targetDir = Path.of("target", "expected", projectKey); | ||
| Files.createDirectories(targetDir); | ||
| for (String language : LANGUAGES) { | ||
| var langDir = Path.of("src", "test", "resources", "expected", language, projectKey); | ||
| if (Files.isDirectory(langDir)) { | ||
| try (var files = Files.list(langDir)) { | ||
| files | ||
| .filter(p -> p.toString().endsWith(".json")) | ||
| .forEach(source -> { | ||
| var targetFile = targetDir.resolve(language + "-" + source.getFileName()); | ||
| try { | ||
| Files.copy(source, targetFile, StandardCopyOption.REPLACE_EXISTING); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Failed to copy expectation file: " + source, e); |
There was a problem hiding this comment.
💡 Bug: mergeExpectedDir keeps stale files from previous runs
mergeExpectedDir only calls Files.createDirectories(targetDir) and copies with REPLACE_EXISTING, so any file left in target/expected/<project>/ from an earlier run survives. On a repeated local run after an expectation file was deleted or renamed (exactly the workflow of updating ruling), the stale <language>-<rule>.json is still handed to LITS as sonar.lits.dump.old and reported as a missing-issue difference, failing the test for a file that is no longer in the repository. Clear the directory before copying.
Delete previously merged files before repopulating the directory.:
private static Path mergeExpectedDir(String projectKey) throws IOException {
var targetDir = Path.of("target", "expected", projectKey);
if (Files.isDirectory(targetDir)) {
try (var stale = Files.list(targetDir)) {
for (var path : stale.toList()) {
Files.deleteIfExists(path);
}
}
}
Files.createDirectories(targetDir);
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
CI failed: TypeScript compilation failure across multiple jobs due to a type mismatch in packages/ruling/testProject.ts where an object is missing properties required by the 'Result' type.OverviewAll 5 analyzed CI jobs failed with a consistent TypeScript compilation error (TS2740) in FailuresTypeScript Compilation Error (confidence: high)
Summary
Code Review
|
1 participant
Summary
its/ruling/src/test/expected/<project>/<language>-<ruleId>.jsontoits/ruling/src/test/resources/expected/<language>/<project>/<ruleId>.jsonlits.ts,testProject.ts) to write and compare results using the new layoutsync-results.mjs,generate-report.mjs,build.yml) to use the new pathsMotivation
Normalize ruling expectation file locations so that it is consistent across all analyzers.
Test plan
npm run ruling)mvn test -Dtest=RulingTest)lits.test.tsunit tests pass🤖 Generated with Claude Code