Merged
Conversation
…atchesDatabase and implement the TODO
Replaced the helper and implemented the TODO in [`ImportExportTests.cs`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs).
### What changed
- Renamed helper:
- `AssertBackupIsRoughlyEqualToDatabase` -> `AssertBackupMatchesDatabase`
- Updated all call sites at:
- [`ImportExportTests.cs:21`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:21)
- [`ImportExportTests.cs:39`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:39)
- [`ImportExportTests.cs:56`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:56)
- [`ImportExportTests.cs:85`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:85)
- Implemented full backup-vs-database assertions in:
- [`ImportExportTests.cs:363`](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:363)
### New assertions now verify
- Days:
- `Guid`, `Date`
- linked `PointGuids` per day
- Categories:
- all key/scalar fields (`Guid`, `Group`, `Name`, `Index`, `ReadOnly`, `Enabled`, `Deleted`, `Type`, medication fields, `Details`)
- linked `PointGuids` per category
- Points:
- all key/scalar fields (`Guid`, `CreatedAt`, `Type`, `Deleted`, `Mood`, `SleepHours`, `ScaleIndex`, `Bool`, `Number`, `Text`, `MedicationDose`)
- linked `DayGuid` and `CategoryGuid`
- Relationship integrity checks:
- each point maps to exactly one day and one category in backup graph
- backup relationship mappings cover all backup points
Also removed the old TODO by implementing those checks.
--- **Lowest-quality tests (ranked) and remove recommendation** 1. **Empty skipped placeholder tests (no assertions, no body)** - [DataIntegrityTests.cs:31](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:31) - [DataIntegrityTests.cs:38](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:38) - [DataIntegrityTests.cs:45](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:45) - [DataIntegrityTests.cs:52](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:52) - Recommendation: **Remove now** (or replace with real tests immediately). These add zero signal and hide gaps. 2. **Test passes on both success and failure paths** - [DataIntegrityTests.cs:404](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:404) `DataPoint_HandlesEmptyGuid` - [DataIntegrityTests.cs:472](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:472) `Category_HandlesVeryLongName` - Recommendation: **Remove or rewrite immediately**. Current structure (`try` + acceptable `catch`) makes them non-diagnostic. 3. **Misnamed/invalid scenario** - [ImportExportTests.cs:316](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ImportExportTests.cs:316) `RestoreDbSets_HandlesOrphanedDataPoints` - Issue: data is not orphaned; point references valid day/category. - Recommendation: **Do not remove**, but **rewrite or rename**. As written it misleads maintainers. 4. **Name and expectation conflict** - [AppDataServiceTests.cs:571](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/AppDataServiceTests.cs:571) `SetPreferences_WithNullPreferenceBackups_HandlesGracefully` - Issue: name says graceful handling, assertion expects `NullReferenceException`. - Recommendation: **Do not remove**, but **fix contract** (either rename to failure case or change implementation/test to truly handle null). 5. **Very weak/trivial assertion** - [DataIntegrityTests.cs:17](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:17) `Day_CannotHaveNullDate` - Issue: asserts default `DateOnly` persists; title implies constraint that is not tested. - Recommendation: **Do not remove**, but **rewrite** to the real invariant you care about (or delete if no invariant exists). 6. **Random-path test with partial/no meaningful assertion by type** - [DataIntegrityTests.cs:173](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/Data/DataIntegrityTests.cs:173) `GetMissingPoints_WithRandom_GeneratesRandomData` - Issue: conditional assertions only for some types; for others it barely verifies anything. - Recommendation: **Keep**, but **make deterministic** and assert full expected fields for a specific category type. 7. **Brittle UI count tests tied to seeded defaults** - [ManageCategoriesTests.razor:157](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ManageCategoriesTests.razor:157) `AddNewCategory` - [ManageCategoriesTests.razor:183](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/ManageCategoriesTests.razor:183) `AddNewMedication` - Issue: hardcoded initial counts (`10`, `3`) are fragile to seed changes. - Recommendation: **Keep**, but assert relative change (`+1`) from initial observed count. 8. **UI assertion too broad** - [DataPointViewTests.razor:40](/mnt/c/Users/danch/source/repos/JournalApp/JournalApp.Tests/DataPointViewTests.razor:40) `Mood_SobEmoji_ShowsMotivationalQuote` - Issue: checks only that *some* snackbar exists. - Recommendation: **Keep**, but assert snackbar message content/source and count delta. If you want, I can make a focused cleanup PR that removes the truly low-signal tests (#1/#2) and rewrites #3/#5/#7 first. ---
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
1 participant
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.
No description provided.