test: simplify alias and arg-parsing tests covered by property tests#182
Merged
test: simplify alias and arg-parsing tests covered by property tests#182
Conversation
Remove example-based tests that are redundant with property-based tests: - alias.test.ts: Remove findCommonWordPrefix and findShortestUniquePrefixes tests (covered by property tests), keep integration tests and specific output verification tests - arg-parsing.test.ts: Remove happy-path type determination tests (covered by property tests), keep error message verification and edge cases Reduction: 295 lines removed, 49 fewer tests Total coverage maintained via property tests
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧
Other
🤖 This preview updates automatically when you update the PR. |
Contributor
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 2175 uncovered lines. Files with missing lines (33)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 70.81% 70.81% —%
==========================================
Files 55 55 —
Lines 7451 7451 —
Branches 0 0 —
==========================================
+ Hits 5276 5276 —
- Misses 2175 2175 —
- Partials 0 0 —Generated by Codecov Action |
BYK
added a commit
that referenced
this pull request
Feb 5, 2026
## Summary Remove example-based tests that are now redundant with property-based tests in `issue-id.property.test.ts`. ## Changes ### `test/lib/issue-id.test.ts` (230 → 144 lines, -86 lines) **Removed** (covered by property tests): - `isNumericId` tests (4 tests) - `isShortSuffix` tests (5 tests) - `isShortId` tests (4 tests) - `expandToFullShortId` verbose examples (1 test) **Kept**: - `parseAliasSuffix` edge case tests (7 tests) - specific parsing behaviors - `expandToFullShortId` representative examples (2 tests) - Integration flow tests (6 tests) - valuable workflow documentation ## Results - **Lines removed**: 86 - **Tests removed**: 14 (949 → 935) - **Coverage**: Maintained via property tests ## Verification All tests pass: ``` 935 pass, 0 fail, 17059 expect() calls ``` --- This is Phase 2 of the test simplification effort. Phase 1 was merged in #182.
BYK
added a commit
that referenced
this pull request
Feb 5, 2026
## Summary Add property-based tests for URL validation and building functions in `src/lib/sentry-urls.ts`. This is Phase 3 of the test improvement effort. ## New Tests ### `test/lib/sentry-urls.property.test.ts` (417 lines, 27 tests) **`isSentrySaasUrl` properties:** - `sentry.io` always returns true - `*.sentry.io` subdomains always return true - Non-sentry.io domains return false - Invalid URLs return false - Security: lookalike domains return false - Deterministic behavior **URL building functions:** - `buildOrgUrl` - output pattern, contains slug, valid URL - `buildProjectUrl` - contains both slugs, valid URL - `buildEventSearchUrl` - event ID in query string - `buildOrgSettingsUrl` - with/without hash fragment - `buildSeerSettingsUrl` - contains /seer/ path - `buildBillingUrl` - with/without product query param **Cross-function properties:** - All URL builders produce valid URLs - All URL builders are deterministic ## Results - **New tests**: 27 - **New assertions**: ~1,800 - **Total tests**: 970 (935 → 970) ## Verification All tests pass: ``` 970 pass, 0 fail, 20318 expect() calls ``` --- ## Test Improvement Summary (All Phases) | Phase | PR | Description | Lines Changed | Tests Changed | |-------|-----|-------------|---------------|---------------| | 1 | #182 ✅ | Simplify alias & arg-parsing tests | -295 | -49 | | 2 | #183 ✅ | Simplify issue-id tests | -86 | -14 | | 3 | #184 🔄 | Add sentry-urls property tests | +417 | +27 | **Net result**: Better test coverage with more robust property-based testing.
3 tasks
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.
Summary
Remove example-based tests that are now redundant with property-based tests, reducing test maintenance burden while maintaining coverage.
Changes
test/lib/alias.test.ts(313 → 132 lines, -181 lines)findCommonWordPrefixtests (8 tests) - all invariants covered by property testsfindShortestUniquePrefixestests (6 tests) - all invariants covered by property testsbuildOrgAwareAliasestests with specific expected outputstest/lib/arg-parsing.test.ts(244 → 109 lines, -135 lines)Results
*.property.test.tsfilesVerification
All tests pass: