Fix key validation and align failing test expectations#3
Open
Cadtastic wants to merge 1 commit into
Open
Conversation
KeyHelper.ValidateKey wrapped ArgumentNullException.ThrowIfNull inside an IsNullOrWhiteSpace check, so empty and whitespace keys were silently accepted while null keys threw the wrong exception type. Split the validation: null now throws ArgumentNullException and empty/whitespace throws ArgumentException with nameof(key) -- matching the .NET BCL convention -- and the XML documentation now lists both exception types. Test fixes: - StateStoreTests: two null-key cases now expect ArgumentNullException (and are renamed to reflect that), matching NullReferenceTests and the new contract. - NullReferenceTests.UpsertAsync_NullKey_ThrowsAsync passed "" instead of null despite its name; corrected to null! so the assertion matches the test's intent. - EdgeCaseTests.Get_NonExistentKey_ThrowsAsync asserted a KeyNotFoundException that StateStoreImplementation.GetAsync has never thrown -- missing keys return default, as verified by GetAsync_ReturnsDefault_WhenKeyDoesNotExist. Renamed and rewritten to verify default(int) is returned, preserving the value-type-specific coverage. All 72 tests pass on net10.0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Stacked on top of #2 (Mongo driver fix) —
mastercan't restore without that PR, so this branch targetsfix/mongodb-driver-security-updateinstead ofmaster. Rebase ontomasterafter #2 merges.Summary
KeyHelper.ValidateKeywas broken. The checkif (string.IsNullOrWhiteSpace(key)) { ArgumentNullException.ThrowIfNull(key, ...); }only ever threw whenkeywasnull— empty and whitespace keys passed validation silently. Replaced withArgumentNullException.ThrowIfNull(key)followed by an explicitIsNullOrWhiteSpacecheck that throwsArgumentExceptionwithnameof(key). XML doc now lists both exception types.StateStoreTestsandNullReferenceTestsdisagreed on what should be thrown for a null key (ArgumentExceptionvs.ArgumentNullException). xUnitThrows<T>requires exact match, so they couldn't both pass. Resolved by following the .NET BCL convention:ArgumentNullExceptionfor null,ArgumentExceptionfor empty/whitespace. The two affectedStateStoreTestscases were renamed and switched to expectArgumentNullException.NullReferenceTests.UpsertAsync_NullKey_ThrowsAsyncwas named "NullKey" but passed"". Corrected to passnull!so input matches intent.EdgeCaseTests.Get_NonExistentKey_ThrowsAsyncassertedKeyNotFoundExceptionthatStateStoreImplementation.GetAsynchas never thrown — missing keys returndefault, asGetAsync_ReturnsDefault_WhenKeyDoesNotExist_Asyncalready verifies. Renamed toGet_NonExistentKey_ReturnsDefault_Asyncand rewritten to assertdefault(int) == 0, preserving the value-type-specific edge case.Test plan
dotnet test— 72/72 passing on net10.0 (was 66/72 before this PR; the 6 failures listed above are fixed).dotnet build— 0 warnings, 0 errors across net8.0 / net9.0 / net10.0.🤖 Generated with Claude Code