Skip to content

SONARJAVA-6211 Upload artifacts if ruling or autoscan fails#5532

Merged
tomasz-tylenda-sonarsource merged 6 commits intomasterfrom
tt/ruling-artifacts
Mar 26, 2026
Merged

SONARJAVA-6211 Upload artifacts if ruling or autoscan fails#5532
tomasz-tylenda-sonarsource merged 6 commits intomasterfrom
tt/ruling-artifacts

Conversation

@tomasz-tylenda-sonarsource
Copy link
Copy Markdown
Contributor

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource commented Mar 25, 2026

To help understand the problem we upload:

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Upload ruling artifacts SONARJAVA-6211 Upload ruling artifacts Mar 25, 2026
@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod Bot commented Mar 25, 2026

SONARJAVA-6211

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource force-pushed the tt/ruling-artifacts branch 3 times, most recently from 611adfa to 2843571 Compare March 25, 2026 15:24
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource changed the title SONARJAVA-6211 Upload ruling artifacts SONARJAVA-6211 Upload artifacts if qa-ruling fails Mar 25, 2026
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource changed the title SONARJAVA-6211 Upload artifacts if qa-ruling fails SONARJAVA-6211 Upload artifacts if ruling-qa fails Mar 25, 2026
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource marked this pull request as ready for review March 25, 2026 16:10
@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented Mar 25, 2026

Summary

Adds a reusable GitHub Action that uploads integration test artifacts and generates an HTML diff report when ruling or autoscan tests fail. This helps developers debug test failures by providing a visual side-by-side comparison of expected vs actual results without needing to run tests locally. The action is then integrated into the CI workflow for both ruling and autoscan jobs.

What reviewers should know

The core logic is in the new .github/actions/upload-actual/action.yml — it's a composite action with three steps: upload raw actual results, generate a diff using Unix diff and diff2html-cli, then upload the HTML report. The build.yml changes are simple applications of this action to the ruling and autoscan jobs, both guarded by if: failure() so they only run when tests fail. Note that diff2html-cli is invoked via npx (requires npm/Node at runtime), and artifacts are retained for 7 days. The action is parameterized to handle different directory structures for ruling vs autoscan.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

Copy link
Copy Markdown
Contributor

@asya-vorobeva asya-vorobeva left a comment

Choose a reason for hiding this comment

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

Generally looks OK. Probably makes sense to upload also artifacts for autoscan tests. Or do it in another PR.

Comment thread .github/workflows/build.yml Outdated
with:
name: actual_${{ matrix.item.runner }}_${{ matrix.item.profile }}
path: its/ruling/target/actual
retention-days: 7
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to configure this policy on repo level.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't have access to do this, but I moved the parameter to a global env in a shared action.

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

Conclusion: Clean, focused CI change. The three new steps work correctly together — the artifact names are safely namespaced by runner/profile, the || true on diff correctly absorbs its non-zero exit code when differences exist, and the if: failure() conditions are appropriate throughout.

🗣️ Give feedback

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource changed the title SONARJAVA-6211 Upload artifacts if ruling-qa fails SONARJAVA-6211 Upload artifacts if ruling or autoscan fails Mar 26, 2026
@tomasz-tylenda-sonarsource
Copy link
Copy Markdown
Contributor Author

Generally looks OK. Probably makes sense to upload also artifacts for autoscan tests. Or do it in another PR.

Done. I extracted a shared action (which is good idea even with one usage).

@sonarqube-next
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@asya-vorobeva asya-vorobeva left a comment

Choose a reason for hiding this comment

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

💯

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource merged commit 91519e8 into master Mar 26, 2026
15 of 16 checks passed
@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource deleted the tt/ruling-artifacts branch March 26, 2026 10:57
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.

2 participants