Repository navigation
refactor: simplify key parsing, naming limits and test setup - #50
Merged
Merged
Conversation
- Share the stored-key parsing loop between StorageWrapper and DeletableStorageWrapper through a ParseKeys iterator, and drop the DeletableStorageWrapper constructor guard that repeated the check made by PersistKeysToStorage. - Derive the SQL Server maximum table name length from the index name pattern instead of a hand-counted constant. - Start one database container per provider for the whole test assembly (xUnit v3 assembly fixtures) instead of one per test class. - Reuse the existing helpers in the PostgreSQL repository tests, share the key material constant, drop unused and duplicated tests, and remove redundant test build settings.
giammin
added a commit
that referenced
this pull request
Oct 22, 2026
xUnit v3 starts assembly fixtures on every run, so since #50 even a filtered unit-only run started the SQL Server and PostgreSQL containers and required Docker. Share each container through a collection fixture instead: it still starts once per provider, but only when that provider's tests run. Move the unique table name helper to the fixtures, so the four test classes no longer keep their own copies.
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.
Code-quality cleanup of the whole codebase (reuse, simplification, efficiency, altitude). No public API or runtime behavior change.
Source
StorageWrapperandDeletableStorageWrappershare the stored-key parsing loop through aParseKeysiterator;TryParseKeyis now private.DeletableStorageWrapperconstructor guard: it repeated the check thatPersistKeysToStoragealready makes before choosing the wrapper.MaxTableNameLengthfrom the index name pattern (GetIndexName) instead of the hand-countedMaxSchemaNameLength - 16. The limit is still 112.Tests
[assembly: AssemblyFixture]) instead of one per test class. This is safe because Respawn captures its table list at fixture start, and the end-to-end tests use their own tables.CreateSutandCountRowshelpers and a newNewTableNamehelper.KeyMaterialthroughTestConstants, remove the unusedTestConstants.Xml, and use the capturing logger factory throughoutStorageWrapperTests.UseDapperDataProtectiontests into a[Theory].TargetFrameworks->TargetFramework; remove the duplicatexunit.runner.visualstudiometadata from the test csproj.Verification
dotnet build -c Release: 0 warnings, 0 errors.dotnet test --no-build -c Release: 156/156 passed.Not done (out of scope)
GetIdshelper in the Dapper package: this needs either new public API orInternalsVisibleToacross separately versioned packages.