test: simplify issue-id tests covered by property tests#183
Merged
Conversation
Remove example-based tests that are redundant with property-based tests: - isNumericId tests (4 tests) - covered by property tests - isShortSuffix tests (5 tests) - covered by property tests - isShortId tests (4 tests) - covered by property tests - expandToFullShortId: simplified to representative examples Kept: - parseAliasSuffix edge case tests (specific parsing behaviors) - Integration flow tests (valuable workflow documentation) Reduction: 86 lines removed, 14 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 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
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 in
issue-id.property.test.ts.Changes
test/lib/issue-id.test.ts(230 → 144 lines, -86 lines)Removed (covered by property tests):
isNumericIdtests (4 tests)isShortSuffixtests (5 tests)isShortIdtests (4 tests)expandToFullShortIdverbose examples (1 test)Kept:
parseAliasSuffixedge case tests (7 tests) - specific parsing behaviorsexpandToFullShortIdrepresentative examples (2 tests)Results
Verification
All tests pass:
This is Phase 2 of the test simplification effort. Phase 1 was merged in #182.