Increase unit test coverage for Support utilities (Steam, Amount, ParseDateString)#7
Open
devin-ai-integration[bot] wants to merge 3 commits into
Open
Conversation
- Created comprehensive unit tests for 3 untested Support utility classes: - SteamBcroundTest.php: 19 test cases for Steam::bcround() method - AmountGetAmountJsConfigTest.php: 26 test cases for Amount::getAmountJsConfig() - ParseDateStringRangeTest.php: 21 test cases for ParseDateString range methods - Modified phpunit.xml to use line coverage instead of path coverage - Changed pathCoverage from true to false to enable pcov driver - Pcov is significantly faster than xdebug for coverage collection - Created GitHub Actions workflow (.github/workflows/sonarcloud.yml) - Runs unit tests with coverage on push to main and pull requests - Uses pcov for fast coverage generation - Uploads coverage results to SonarCloud Test Results: - All 439 unit tests pass (66 new tests added) - Coverage increased from 0.0% to 1.82% (1,044 covered elements) - Coverage report generated successfully in coverage.xml This establishes the foundation for ongoing coverage tracking and improvement in the codebase. Co-Authored-By: Sam Fertig <sam.fertig@codeium.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
The workflow was failing during composer install because Laravel's post-install script (firefly-iii:instructions install) requires APP_KEY and other environment variables to be set. Added environment setup step before composer install with: - APP_ENV=testing - APP_KEY (dummy key for CI) - DB_CONNECTION=sqlite - DB_DATABASE (empty for SQLite) - APP_LOG_LEVEL=notice - AUDIT_LOG_LEVEL=emergency This follows standard Laravel CI practices and matches the environment variables configured in phpunit.xml. Co-Authored-By: Sam Fertig <sam.fertig@codeium.com>
The workflow was failing during test execution because Laravel was trying to access a SQLite database file at storage/database/database.sqlite that doesn't exist in the CI environment. Changed DB_DATABASE from empty string to ':memory:' which uses an in-memory SQLite database for tests. This is the standard approach for Laravel CI testing and avoids filesystem dependencies. This is the 2nd fix for the SonarCloud workflow: - 1st fix: Added APP_KEY and other required Laravel env vars (successful) - 2nd fix: Changed DB_DATABASE to :memory: for tests (this commit) Co-Authored-By: Sam Fertig <sam.fertig@codeium.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to Devin run: https://app.devin.ai/sessions/321f0ee3ac014cbe994f3803fdae8be1
Requested by: Sam Fertig (sam.fertig@cognition.ai), GitHub: @samfert-codeium
Summary
This PR addresses the 0% test coverage reported by SonarQube by adding comprehensive unit tests for three previously untested Support utility classes and establishing CI integration for ongoing coverage tracking.
Changes in this pull request
Added 66 new unit tests across 3 test files (total tests: 373 → 439):
SteamBcroundTest.php: 19 tests for theSteam::bcround()method covering null/empty inputs, positive/negative decimals, scientific notation, and edge casesAmountGetAmountJsConfigTest.php: 26 tests for theAmount::getAmountJsConfig()static method covering all 5 sign position modes (0-4), spacing variants, and currency symbol positioningParseDateStringRangeTest.php: 21 tests forParseDateString::isDateRange()andparseRange()methods covering day/month/year ranges and combinationsModified
phpunit.xmlto changepathCoveragefromtruetofalse:Created GitHub Actions workflow (
.github/workflows/sonarcloud.yml):SONAR_TOKENsecret to be configured (should already exist from PR Set up SonarQube integration for local analysis #5)Test Coverage Improvement
Review Checklist
Test correctness: Verify the test assertions match expected behavior, especially:
Amount::getAmountJsConfig()formatting for all sign position modesSteam::bcround()behavior with edge cases (null, empty, scientific notation)ParseDateStringrange parsing logicConfiguration change: Confirm that disabling path coverage (
pathCoverage="false") is acceptable given the performance trade-offCI setup: Verify
SONAR_TOKENsecret is configured in repository settings (workflow will fail without it)Scope: This PR provides a foundation with 1.82% coverage. Consider whether additional test coverage should be required before merge or can be addressed in follow-up PRs.
Testing
Note: This establishes the foundation for ongoing coverage tracking. Many Support utility classes remain untested and will require additional test coverage in future PRs.