Skip to content

fix: Add SonarCloud test configuration and refactor repository tests#37

Merged
hsc00 merged 6 commits intomainfrom
fix/quality-gates-issues
Apr 15, 2026
Merged

fix: Add SonarCloud test configuration and refactor repository tests#37
hsc00 merged 6 commits intomainfrom
fix/quality-gates-issues

Conversation

@hsc00
Copy link
Copy Markdown
Owner

@hsc00 hsc00 commented Apr 15, 2026

Summary

This PR addresses quality gates issues across the CI/CD pipeline and test infrastructure by:

  1. SonarCloud Configuration — Extended the SonarQube scan step with test discovery parameters (-Dsonar.tests=tests and -Dsonar.test.inclusions=tests/**) to ensure the tests directory is properly analyzed during quality scanning.

  2. Trivy Security Scanning — Updated Trivy CLI from v0.69.2 to v0.69.3 and refined container image scanning to focus on vulnerability scanning only (--scanners vuln) with severity filtering to CRITICAL level (previously HIGH,CRITICAL).

  3. Test Infrastructure Refactoring — Consolidated database test harness by:

    • Changing BaseAssetsTableTestCase::$selectedConnection visibility from private to protected to enable subclass access
    • Adding optional $applyMigration parameter to withTemporarySchema() to conditionally apply migrations during test setup
  4. MySQLAssetRepositoryTest Consolidation — Removed 205 lines of redundant local database bootstrap code by migrating to BaseAssetsTableTestCase, introducing reusable test helpers (createRepository(), assertFoundByIdAndUploadId(), assertPersistedSingleRowMatches()), and extracting constants (STALE_ASSET_WRITE_MESSAGE, timestamp constants).

  5. Test Quality Markers — Added // NOSONAR inline comments to test data providers in UploadTargetTest.php to suppress false positives on intentional insecure URLs and localhost patterns used for local development testing.

  6. AssetIdTest Enhancement — Introduced a private createAssetId() helper method for test instantiation to maintain consistency with code organization patterns.

Net Impact: -205 lines in MySQLAssetRepositoryTest (644 lines → centralized via base class), +2 lines in CI workflow, +3 lines in Trivy workflow, improved test maintainability, and quality gate compliance through proper configuration and annotation.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

Warning

Rate limit exceeded

@hsc00 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 51 minutes and 40 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 40 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea6fb58f-421a-4869-a9be-3eaf590f57d9

📥 Commits

Reviewing files that changed from the base of the PR and between d792a8a and fec919a.

📒 Files selected for processing (1)
  • tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php
📝 Walkthrough

Walkthrough

CI and test infrastructure updates: SonarCloud test configuration added to CI workflow, Trivy scanner updated and configured to check critical vulnerabilities only, test base class extended with conditional migration support, and integration tests refactored to leverage shared base class infrastructure instead of duplicated database management code.

Changes

Cohort / File(s) Summary
CI/CD Workflows
.github/workflows/ci.yml, .github/workflows/trivy.yml
Added SonarCloud test discovery parameters to SonarQube scan step; upgraded Trivy from v0.69.2 to v0.69.3 and configured container scanner to check only CRITICAL severity vulnerabilities.
Test Base Infrastructure
tests/Integration/Infrastructure/Persistence/BaseAssetsTableTestCase.php
Increased $selectedConnection visibility to protected for subclass access; extended withTemporarySchema() with optional $applyMigration parameter to conditionally skip migration execution.
Test Class Refactoring
tests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.php
Migrated from extending TestCase to BaseAssetsTableTestCase; consolidated database setup via new createRepository() helper; replaced inline assertions with reusable assertFoundByIdAndUploadId() and assertPersistedSingleRowMatches() helpers; removed duplicated database bootstrap and connection management logic; centralized expected constants for timestamps and messages.
Test Helper Methods & Comments
tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php, tests/Unit/Domain/Asset/ValueObject/UploadTargetTest.php
Added private createAssetId() helper method to AssetIdTest; added NOSONAR inline comments to test data provider entries in UploadTargetTest for HTTP, FTP, and localhost URL fixtures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • feat: domain model #34: Both PRs modify unit test files under tests/Unit/Domain/Asset/ValueObject/ (AssetIdTest.php and UploadTargetTest.php), indicating related test maintenance work.

Poem

🐰 The warren's tests now share a common burrow,
No more tunnels dug in every warren tomorrow,
Base classes firm, helpers reusable and bright,
Security scanning set tighter at night,
Refactoring hops make our tests lean and light! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely missing; no content was provided despite a clear template being available in the repository. Add a complete pull request description following the provided template, including a summary of changes, list of modified files, testing details, and any reviewer notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 17.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: SonarCloud test configuration additions and significant refactoring of repository tests across multiple test files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/quality-gates-issues

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php`:
- Around line 83-87: The helper method createAssetId that simply returns new
AssetId($value) is unnecessary indirection; remove the private function
createAssetId(string $value): AssetId and replace its usages in tests with
direct instantiation new AssetId($value) (e.g., in tests like
itReturnsSameValueWhenConstructedWithValidUuidV4) for clarity — if you must keep
it for static analysis or coverage, add a brief comment above createAssetId
explaining why it exists.
- Line 66: Remove the trailing whitespace on the blank/affected line in the
AssetIdTest class (AssetIdTest.php) that is causing PHP CS Fixer to fail; open
the AssetIdTest file, locate the line near the test methods in class AssetIdTest
(around the test declarations), delete any trailing spaces at end-of-line, save,
and re-run the linter/CI to confirm the whitespace-only change resolves the
failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f3f447a5-881f-4068-9962-1ab64ade452c

📥 Commits

Reviewing files that changed from the base of the PR and between bbb6208 and d792a8a.

📒 Files selected for processing (6)
  • .github/workflows/ci.yml
  • .github/workflows/trivy.yml
  • tests/Integration/Infrastructure/Persistence/BaseAssetsTableTestCase.php
  • tests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.php
  • tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php
  • tests/Unit/Domain/Asset/ValueObject/UploadTargetTest.php

Comment thread tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php Outdated
Comment thread tests/Unit/Domain/Asset/ValueObject/AssetIdTest.php
@hsc00 hsc00 changed the title fix: Quality gates issues fix: Add SonarCloud test configuration and refactor repository tests Apr 15, 2026
@hsc00 hsc00 merged commit 6312775 into main Apr 15, 2026
7 checks passed
@hsc00 hsc00 deleted the fix/quality-gates-issues branch April 15, 2026 12:50
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