Skip to content

SC-49820 Severity metrics decoration - #583

Merged
antoine-vinot-sonarsource merged 4 commits into
masterfrom
task/orlov/SC-49820/severity-metrics
Jun 15, 2026
Merged

SC-49820 Severity metrics decoration#583
antoine-vinot-sonarsource merged 4 commits into
masterfrom
task/orlov/SC-49820/severity-metrics

Conversation

@OrlovAlexander85

Copy link
Copy Markdown
Contributor

Adding formatters for new Severity Metrics Conditions

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jun 5, 2026

Copy link
Copy Markdown

SC-49820

@OrlovAlexander85
OrlovAlexander85 force-pushed the task/orlov/SC-49820/severity-metrics branch from 4165390 to a306406 Compare June 5, 2026 13:53
@OrlovAlexander85
OrlovAlexander85 force-pushed the task/orlov/SC-49820/severity-metrics branch from a306406 to 022082c Compare June 5, 2026 14:06

@stanislavhh stanislavhh left a comment

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.

Overall looks good, small suggestion, and given that we introduced new measure type, it will be a good idea to verify that measure is rendered in html as expected (comparator, measure name)

Comment thread src/common/latest/helpers/measures.ts Outdated
Comment thread src/common/latest/sonarqube/__tests__/HtmlAnalysisReport-test.ts
Comment thread src/common/latest/helpers/measures.ts
@OrlovAlexander85
OrlovAlexander85 force-pushed the task/orlov/SC-49820/severity-metrics branch from 184aa3e to f44cf6b Compare June 9, 2026 07:49
@sonarqube-next

sonarqube-next Bot commented Jun 9, 2026

Copy link
Copy Markdown

@OrlovAlexander85
OrlovAlexander85 marked this pull request as ready for review June 9, 2026 08:15

@stanislavhh stanislavhh left a comment

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.

LGTM

Comment thread src/common/sonarcloud-v3/helpers/measures.ts Outdated
Comment thread src/common/latest/helpers/measures.ts

@antoine-vinot-sonarsource antoine-vinot-sonarsource left a comment

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.

@OrlovAlexander85 Quick heads-up. I'm trying to validate the changes from a functional point of view, because our ITs don't handle the fail QG scenario I believe. However I faced some issues (not related to your changes) when trying it on a Dev environment.
Please bear with me a little bit more 🙏

@antoine-vinot-sonarsource antoine-vinot-sonarsource left a comment

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 tried it out and the change works for the formatted evaluation at the start of the line. However, the required threshold is not properly formatted because it shows "one less" than the value associated to the given severity.
See my screenshot below: I set up the QG to INFO level, hence it show required <= 4 and not required < INFO (Or whatever we want there)

Image

@OrlovAlexander85

Copy link
Copy Markdown
Contributor Author

@antoine-vinot-sonarsource Thanks a lot for doing functional validation.
I will add a proper mapping to a condition as well

@antoine-vinot-sonarsource antoine-vinot-sonarsource left a comment

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.

LGTM!

Image

@OrlovAlexander85 The pipeline failed because of some timeout when publishing the test extension. It's a known thing for which I did not find a solution yet sadly. You might need to retrigger a new pipeline, as rerunning the failed one won't work I think.

@OrlovAlexander85
OrlovAlexander85 force-pushed the task/orlov/SC-49820/severity-metrics branch from a90d658 to 86bfb5e Compare June 15, 2026 07:39
@sonarqube-next

Copy link
Copy Markdown

@sonarqube-next

Copy link
Copy Markdown

@OrlovAlexander85

Copy link
Copy Markdown
Contributor Author

Hey @antoine-vinot-sonarsource , once again thanks a lot for validation. I rerun the pipeline and all checks are green and PR is ready to be merged.
Can you please merge it, as I do not have required permissions on this repo.

@antoine-vinot-sonarsource
antoine-vinot-sonarsource merged commit f20a2bb into master Jun 15, 2026
36 checks passed
@antoine-vinot-sonarsource
antoine-vinot-sonarsource deleted the task/orlov/SC-49820/severity-metrics branch June 15, 2026 07:59
@gitar-bot

gitar-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Implements severity metrics decoration for project analysis, though the removal of existing tests leaves this feature currently untested.

✅ 1 resolved
Quality: Severity formatter tests and snapshots removed, leaving feature untested

📄 src/common/latest/sonarqube/tests/HtmlAnalysisReport-test.ts 📄 src/common/sonarcloud-v3/sonarqube/tests/HtmlAnalysisReport-test.ts 📄 src/common/sonarqube-v7/sonarqube/tests/HtmlAnalysisReport-test.ts 📄 src/common/latest/helpers/measures.ts:175-189
The most recent commit (Revert "SC-49820 PR review") removes the unit tests and snapshots that exercised the newly added severity-metrics decoration. Across all three modules it deletes the "should render full HTML for standard severity condition..." and "should render full HTML for MQR severity condition..." tests (56 lines each in HtmlAnalysisReport-test.ts) plus the associated 78-line .snap snapshot files. These were the only tests covering severityFormatter, severityMqrFormatter, and getEffectiveMetricType / the SEVERITY and SEVERITY_MQR formatter wiring in measures.ts.

The production code that maps severity values (5/10/15/20/25 → INFO/MINOR/.../BLOCKER and MQR labels) and resolves metric types still ships, but it now has no automated coverage, so regressions (e.g. an incorrect label-key mapping or a broken metricKey lookup) would go undetected. If the revert was intended only to drop the parseIntNumber refactor, the test/snapshot deletions appear to be unintended collateral. Restore the severity formatter tests and snapshots before merging.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

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.

3 participants