-
Notifications
You must be signed in to change notification settings - Fork 1
Test: Consolidation Plan #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Baseline coverage captured from workflow run 21102796115 (2026-01-17): Unit Test Coverage: - Instructions: 25.97% (20,646 / 79,496) - Branches: 21.80% (1,235 / 5,664) - Lines: 24.84% (3,688 / 14,849) - Methods: 34.30% (959 / 2,796) Integration Test Coverage: - Instructions: 43.79% (34,814 / 79,496) - Branches: 31.07% (1,760 / 5,664) - Lines: 45.34% (6,732 / 14,849) - Methods: 46.07% (1,288 / 2,796) This establishes a baseline for Issue #174 work on test suite consolidation. Co-authored-by: wharris <wharris@upscalews.com>
Analysis of ~38,000 lines of test code identified: - ~5,000 lines in deprecated_raw_calendarmonitor/ to remove - ~20 duplicate test pairs (Robolectric + instrumented) to consolidate - Large tests (700+ lines) to simplify Estimated reduction: ~10,900 lines (28% reduction) Target: ~27,000 lines while maintaining coverage baseline Co-authored-by: wharris <wharris@upscalews.com>
Code Coverage Report
|
📊 Code Coverage Summary
|
docs/test-consolidation-plan.md
Outdated
|
|
||
| ### Key Findings | ||
|
|
||
| 1. **Massive Duplication**: ~20 test classes exist as BOTH Robolectric (unit) AND instrumented (integration) versions with nearly identical tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplication is intentional. There are some things that roboelectric cannot test for instance anything to do with sqlite because
- we use a version that has custom extensions
- Even if we didn't, I heard that there. Their sequel light support is rudimentary at best so given how important dealing with storage is in this application, I don't trust testing it there
But robo electric is great for quick sanity checks for application functionality overall.
The way I have it set up is only the roboelectric test block PRS.
But releases don't if the Android instrumentation tests fail
docs/test-consolidation-plan.md
Outdated
| ### Key Findings | ||
|
|
||
| 1. **Massive Duplication**: ~20 test classes exist as BOTH Robolectric (unit) AND instrumented (integration) versions with nearly identical tests | ||
| 2. **Deprecated Tests**: ~5,000 lines in `deprecated_raw_calendarmonitor/` folder that should be removed or archived |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really paranoid here. At this point the fixtures have been around for almost a year so I trust them a lot but it's a little bit harder to follow exactly what's going on in them.
In the deprecated test, it's pretty straightforward to follow exactly what's happening.
And they test the app core application functionality (that it that we're watching the calendar properly to get events. If this breaks nothing else works)
Maybe there's a refactoring of the fixtures that we could do that would make them easier to follow. Maybe there's some kind of fixture framework we should be using or something like that then I would feel more comfortable getting rid of them
docs/test-consolidation-plan.md
Outdated
|
|
||
| 1. **Massive Duplication**: ~20 test classes exist as BOTH Robolectric (unit) AND instrumented (integration) versions with nearly identical tests | ||
| 2. **Deprecated Tests**: ~5,000 lines in `deprecated_raw_calendarmonitor/` folder that should be removed or archived | ||
| 3. **Testing Overly Simple Logic**: Some tests verify trivial functionality that could be simplified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agree here
docs/test-consolidation-plan.md
Outdated
| 1. **Massive Duplication**: ~20 test classes exist as BOTH Robolectric (unit) AND instrumented (integration) versions with nearly identical tests | ||
| 2. **Deprecated Tests**: ~5,000 lines in `deprecated_raw_calendarmonitor/` folder that should be removed or archived | ||
| 3. **Testing Overly Simple Logic**: Some tests verify trivial functionality that could be simplified | ||
| 4. **Large Monolithic Tests**: Several test files exceed 700+ lines and could be split or simplified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agree here also
docs/test-consolidation-plan.md
Outdated
|
|
||
| --- | ||
|
|
||
| ### Phase 2: Consolidate Duplicate Test Pairs (~5,000-7,000 lines reduction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of paranoid here too. Don't really want to see the coverage number go down in the instrumentation test, especially since we don't block PRS on them
Key changes based on @williscool's review: - KEEP Robolectric + Instrumented pairs (intentional design) - Robolectric: quick PR-blocking sanity checks - Instrumentation: real Android verification for releases - SQLite/Room MUST use instrumentation (custom extensions) - DEFER deprecated test removal - Tests are clearer than fixtures - Test core calendar monitoring (critical path) - Prerequisite: improve fixture clarity first - FOCUS on safe improvements: - Remove overly simple tests - Refactor large monolithic tests - Improve fixture documentation Revised estimate: ~1,000-2,500 lines (3-7%) vs original ~10,900 (28%) Quality > quantity for test consolidation Co-authored-by: wharris <wharris@upscalews.com>
Research comparing current test setup against industry best practices: Current Setup: - JUnit 4, MockK, Robolectric 4.16, Ultron, Espresso - Custom fixtures (BaseCalendarTestFixture, MockCalendarProvider, etc.) Quick Wins Identified: - Add Truth assertions for better failure messages - Use ShadowAlarmManager instead of manual mocking - Create Object Mother classes for test data - Verify using PAUSED looper mode in Robolectric Medium Effort: - Formalize Robot Pattern for UI tests - Extract Test Data Builders from fixtures - Document fixture usage with examples Investigated but not recommended: - Hilt Testing (requires architecture changes) - JUnit 5 (limited Android support) - Moving SQLite tests to Robolectric (custom extensions need real DB) Includes sample Object Mother implementation for EventAlertRecord. Co-authored-by: wharris <wharris@upscalews.com>
📊 Code Coverage Summary
|
📊 Code Coverage Summary
|
Quick Wins Implemented: 1. Truth assertions (com.google.truth:truth:1.4.2) - Added for both unit and instrumented tests - Better failure messages and fluent API 2. Object Mother pattern for test data - EventMother.kt - EventAlertRecord factory methods - MonitorAlertMother.kt - MonitorEventAlertEntry factory methods - Reduces boilerplate, centralizes test data creation 3. Robolectric PAUSED looper mode - Created robolectric.properties with explicit config - looperMode=PAUSED for realistic async testing - sdk=34 for Android 14 behavior 4. ShadowAlarmManager helper - AlarmManagerTestHelper.kt for cleaner alarm testing - Provides introspection into scheduled alarms - Alternative to MockK mocking for AlarmManager Updated consolidation plan with: - Documentation of implemented quick wins - Future improvements section (Robot Pattern, Gradle Managed Devices, etc.) - Not recommended items with reasons Co-authored-by: wharris <wharris@upscalews.com>
📊 Code Coverage Summary
|
Note
Introduces documentation for current test coverage and a roadmap to reduce test bloat.
docs/coverage-baseline/withBASELINE-SUMMARY.md,unit-test-coverage.csv, andintegration-test-coverage.csv(Unit: 25.97% lines, Integration: 45.34% lines; 334 classes analyzed)docs/test-consolidation-plan.mdoutlining phased deletions and merges of duplicate/legacy tests, fixture consolidation, and refactors targeting ~10.9k LOC reduction (~28%) while maintaining coverage at or above baselineWritten by Cursor Bugbot for commit 24a9c18. This will update automatically on new commits. Configure here.