Skip to content

fix(sonar): exclude tests from duplication metrics - #4284

Merged
kwakayama merged 2 commits into
mainfrom
fix/sonar-new-code-duplication
Aug 28, 2026
Merged

fix(sonar): exclude tests from duplication metrics#4284
kwakayama merged 2 commits into
mainfrom
fix/sonar-new-code-duplication

Conversation

@kwakayama

@kwakayama kwakayama commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Exclude test/spec files and the tests/ tree from Sonar CPD duplication metrics.
  • Keep those files in sonar.sources so Sonar issues remain visible and coverage configuration is unchanged.
  • Restore the New Code duplication gate after the current main analysis counted repetitive test fixtures and assertions as production duplication.

Evidence

SonarCloud main currently reports:

  • New Code duplication: 8.716% (threshold: 3%)
  • New Code coverage: 85.95% (threshold: 80%)
  • Reliability, security, maintainability, and hotspot conditions: passing

The current analysis attributes 17,938 of 18,526 duplicated New Code lines to test files. Non-test code accounts for 588 duplicated lines across 64,651 New Code lines (0.909%).

Verification

  • deno fmt --check
  • deno task lint
  • git diff --check
  • Sonar properties CPD exclusion contract

The local pre-push typecheck was blocked by pre-existing Deno/Node compatibility errors in the baseline checkout; the required CI typecheck remains authoritative.

Summary by CodeRabbit

  • Chores
    • Updated code-quality analysis settings to exclude test files from copy-paste detection while continuing to include them in overall source analysis.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-08-28T21:44:06.104263Z 50e8ec4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 287 2217 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 54 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c0a103ce-2f18-4336-8c18-4896dbe383e6

📥 Commits

Reviewing files that changed from the base of the PR and between 31d9a79 and 50e8ec4.

📒 Files selected for processing (1)
  • sonar-project.properties

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6bdcccaf-ba06-479f-9598-9e4cbe237db9

📥 Commits

Reviewing files that changed from the base of the PR and between e809801 and 31d9a79.

📒 Files selected for processing (1)
  • sonar-project.properties

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

SonarQube analysis configuration

Layer / File(s) Summary
Configure test-file CPD exclusions
sonar-project.properties
Documents test-file handling and excludes test and spec files plus tests/** from SonarQube CPD analysis.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 31d9a

This change excludes test and spec files from duplication metrics while preserving issue visibility and coverage configuration; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: kojiwakayama

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: excluding test files from SonarQube duplication metrics.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sonar-new-code-duplication

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread sonar-project.properties Outdated
@gitar-bot

gitar-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

Note

Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime.
Learn more

Code Review ✅ Approved 1 resolved / 1 findings

Excludes test and spec files from Sonar CPD duplication metrics to restore the New Code duplication gate, reducing reported duplication from 8.716% to approximately 0.909% in non-test code. Consider expanding the exclusion patterns to cover fixture, mock, and helper files (**/*.fixture.*, **/*.mock.*, **/__fixtures__/**, **/testing/**) that may reintroduce intentional test duplication later.

✅ 1 resolved
Quality: CPD exclusions miss fixture/mock/helper test files

📄 sonar-project.properties:11
The pattern **/*.test.*,**/*.spec.*,tests/** covers .test.ts files at any depth, but the repo has no .spec. files and also has repetitive test-support files that use other naming (e.g. *.test-helpers.ts, *.fixture.ts, *.mock.ts, and files under __fixtures__/, fixtures/, testing/) which *.test.* does not match. Those files can reintroduce intentional test duplication into the New Code CPD gate later. Consider aligning with the coverage exclusions and adding patterns such as **/*.fixture.*,**/*.mock.*,**/__fixtures__/**,**/testing/** (the **/*.spec.* entry can be dropped since no spec files exist).

Options

Display: compact → Showing less information.

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

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@kwakayama

Copy link
Copy Markdown
Contributor Author

Addressed the review suggestion in commit 50e8ec4f0f6a6f93be8b7347229810a161f28a5d.

The CPD exclusion now also covers *test-helpers.*, *.fixture.*, *.mock.*, __fixtures__/, fixtures/, and testing/ support paths. These paths are test support in this repository. They remain in sonar.sources, so Sonar issue analysis is unchanged; only duplication calculation excludes them. The existing tests/** exclusion continues to cover the full integration and end-to-end test tree.

Verified the current repository has no duplicated New Code outside these test-support conventions that would be hidden by this change.

@github-actions

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 50e8ec4f0f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@sonarqubecloud

Copy link
Copy Markdown

@kwakayama

Copy link
Copy Markdown
Contributor Author

Codex Review — APPROVE

Reviewed SHA: 50e8ec4f0f6a6f93be8b7347229810a161f28a5d against base e80980138518be01038462cc6fd59858719ceddb.

No actionable findings.

  • Sonar semantics/scope: sonar.cpd.exclusions affects only duplication calculation; sonar.sources=. and sonar.exclusions=coverage-profiles/** remain unchanged, so matched files stay in source analysis and production issue/security findings are not suppressed. This matches Sonar's documented parameter contract.
  • Pattern audit: Against the exact-head tree, the patterns match 2,648 unique files, all under current test/spec naming, test-helper/fixture/mock conventions, test-support directories (__fixtures__, fixtures, testing), or tests/**. The matched src/testing/**, cache/repository testing modules, and fixture trees are explicitly test-support code; no current production implementation path is swept into CPD exclusions.
  • Prior concern: Addressed by 50e8ec4: the second commit adds *test-helpers.*, *.fixture.*, *.mock.*, __fixtures__/**, fixtures/**, and testing/**. The earlier inline thread is outdated, though still unresolved in GitHub.
  • Security/maintainability: The change is limited to one documented analysis property; no secrets, runtime behavior, source exclusion, fallback, or broad failure masking is introduced.

Verification: exact pinned diff and repository guidance inspected; git diff --check passed; exact-head CI format, lint, typecheck, test-layout, coverage gate, CodeQL JavaScript/TypeScript, Sonar scan, and SonarCloud Quality Gate passed. SonarCloud records commit 50e8ec4f0f6a6f93be8b7347229810a161f28a5d with 0 new bugs, vulnerabilities, and code smells. No properties-file LSP is available/applicable; exact-head CI typecheck is green. Five broader runtime/integration/CodeQL jobs were still pending at posting and are not represented as passed here.

Review-Gate:
Reviewer: Codex
Reviewed-SHA: 50e8ec4
Score: 98/100
Actionable-Findings: 0
Verdict: APPROVE

@kwakayama
kwakayama added this pull request to the merge queue Aug 28, 2026
Merged via the queue into main with commit f6b8d82 Aug 28, 2026
58 checks passed
@kwakayama
kwakayama deleted the fix/sonar-new-code-duplication branch August 28, 2026 22:09
kojiwakayama added a commit that referenced this pull request Sep 3, 2026
Supplying `urlTransform` replaces react-markdown's `defaultUrlTransform`,
so `sanitizeUrl` is the only scheme guard left in a scaffolded chat app.
It anchored both regexes at offset 0 over the raw string, but browsers
ignore ASCII spaces and control characters while parsing a URL: a
destination written as `[x](java	script:alert(1))` reaches the
renderer as `java\tscript:alert(1)`, matches neither regex, and still
navigates to `javascript:` when clicked. ` data:text/html,...` and a
leading control character bypass it the same way. Match against a copy
with those characters removed so an obfuscated scheme fails closed.

The `img` override also emitted an anchor. Markdown allows a linked
image (`[![alt](src)](href)`), which react-markdown renders through the
`a` override, so that anchor nested inside another one - invalid HTML
the browser repairs into a different tree than React rendered, which
mismatches on hydration. Render inert text carrying the source in
`title` instead; the surrounding link, when there is one, still works.

templates/index.test.ts now runs the policy rather than grepping for it:
it lifts `sanitizeUrl` out of each scaffolded renderer and asserts the
obfuscated schemes are dropped and ordinary URLs survive. Each of the
four bypasses above fails against the previous implementation. It also
asserts the `img` override emits no anchor.

Sonar's duplication gate reported 85.3% duplicated new lines because the
five chat starters scaffold the same `app/markdown-renderer.tsx`. That
parity is structural: every starter is copied verbatim into a generated
project, so one starter's file cannot import a module from a sibling.
Exclude `templates/files/**` from CPD, alongside the test-fixture
exclusions added for the same reason in #4284. The files stay in
`sonar.sources`, so their bugs, smells, and security issues are still
reported.

Claude-Session: https://claude.ai/code/session_01QfWNMiUhvWMKWi6BGfVdY3
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