T4 follow-up: negative-int + MinValue/MaxValue edge cases#135
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds incremental unit-test coverage for IsBetween (exclusive) and IsInRange (inclusive) to lock in behavior for negative integer ranges and int.MinValue/int.MaxValue, plus a clarifying comment on the Money test fixture’s CompareTo(null) convention.
Changes:
- Add negative-bound integer
[Theory]coverage for bothIsBetweenandIsInRange. - Add
Facts coveringint.MinValue/int.MaxValueagainst full-range bounds for both methods. - Document the
Money.CompareTo(null)convention used in customIComparable<T>tests.
4635e65 to
0b23a18
Compare
1f902dd to
decd9d1
Compare
decd9d1 to
68ebca7
Compare
The bulk of the T4 audit landed in #134 (custom IComparable<T> tests + null-bound contract tests). This follow-up closes the smaller edge-case gaps the audit flagged but skipped at the time: 1. Negative integer range — every existing int Theory used positive bounds (1, 10). Added matching Theories with bounds (-10, 0) for both IsBetween and IsInRange covering below, exact-lower, mid, exact-upper, above. By construction these exercise the same code path as the positive cases, but a future refactor introducing an Abs-based shortcut or an unsigned compare would now fail. 2. int.MinValue / int.MaxValue boundaries — added four Facts covering value=int.MaxValue and value=int.MinValue against (MinValue, MaxValue) bounds for both methods. Confirms that strict bounds reject the exact extremes (IsBetween → false) and inclusive bounds accept them (IsInRange → true). 3. Money record's CompareTo(null) convention documented with a comment explaining the choice ('null < any value', per the IComparable<T> guidance). 72 tests pass on net10.0 (58 after #134 + 14 new rows / Facts here).
68ebca7 to
5da64c9
Compare
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
Follow-up to PR #134 (T4 test-quality audit). Closes the smaller
edge-case gaps the audit noted but skipped at the time — the bulk
(custom
IComparable<T>+ null-bound contracts) landed in #134;this PR is purely additive coverage.
Stacked on #134
Base =
stack/t4-test-quality-audit. When #134 merges, this PR'sbase auto-promotes to
vNext.What this adds
1. Negative integer range Theories (10 rows)
Every existing int Theory used positive bounds (1, 10). Added a
parallel Theory with bounds (-10, 0) covering below, exact-lower,
mid, exact-upper, above for both methods. By construction these
exercise the same code path as the positive cases, but a future
refactor introducing an
Abs-based shortcut or an unsigned comparewould now break loudly.
2.
int.MinValue/int.MaxValueboundary Facts (4)Four Facts covering
value = int.MaxValueandvalue = int.MinValueagainst
(int.MinValue, int.MaxValue)bounds for both methods.Confirms strict bounds reject the exact extremes (
IsBetween → false)and inclusive bounds accept them (
IsInRange → true).3.
Moneyrecord documentation (comment only)The
Moneyfixture introduced in #134 stipulates thatCompareTo(null)returns +1 (null is the smallest value). That's avalid convention per IComparable guidance but the choice was
hidden in the record definition. Added a comment explaining the
rationale.
Skipped from the audit notes — not worth adding
list): behaviour is type-agnostic, adding string/DateTime/char rows
would be padding.
"", very long strings) (Change license from Mozilla Public to MIT #5): lexicographiccomparison is well-defined; gain is small.
and the typed-DateTime variant exercise the same behaviour; removing
either loses nothing but adds churn.
without a mock, not a real risk.
Verified
72 cases — 58 after #134 + 10 negative-int rows + 4 MinValue/MaxValue
Facts.