Refuse every homemade TimeProvider, not only one named FakeTimeProvider - #569
Merged
Merged
Conversation
A test that wrote its own clock under another name passed the check, which looked only at the name. NoHomemadeClockTests now refuses any type in the built assemblies that derives from TimeProvider, so a copy under any name, generic or file-local, is found. The clocks it found are gone. Every fixed or shifted clock becomes a FakeTimeProvider instance, and the key ring tests that needed a signal when the refresh loop armed its timer now advance the clock until a load arrives and wait until the count settles, so each case still sees exactly one load per advance. A comment in ClientAssertionMayTightenItselfTests no longer says a reservation cannot be given back; the replay cache can release one, and it is this path that never does.
The server unit tests now reference every other project of the solution without referencing their assemblies, so building them alone, as the continuous integration shard does, builds everything the clock check reads; a second row fails while a solution project is not reached. The fixed clocks built with Moq, which a check over built assemblies cannot see, are FakeTimeProvider instances, and the remark says what the check misses. The check itself gets rows proving it recognizes a clock. The key ring refresh tests cap how far they advance the clock while arming the loop, and wait for the logged failures rather than only for the loads before counting them.
|
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.
The check against homemade clocks looked only at the name
FakeTimeProvider, so a test that wrote its own clock under another name passed.NoHomemadeClockTestsnow refuses any type in the built assemblies that derives fromTimeProvider. A copy under any name, generic or file-local, is found. A second row proves the check itself recognizes a clock.It found several test clocks, and they are all gone:
FakeTimeProviderstarted at the same moment. So are the fixed clocks that were built with Moq: a mock is created while the tests run, in an assembly no build writes, so the check cannot see it, and the class remark says so.Continuous integration builds and tests each test project on its own, so the check could only see what the server unit tests depend on. That project now references every other project of the solution with
ReferenceOutputAssembly="false". Building it alone therefore builds the whole solution first, without making any of it visible to the test code. A row fails if a project ofAbblix.Oidc.slnxis not reached through those references, and the paths are compared case-sensitively, the way a Linux runner resolves them.A comment in
ClientAssertionMayTightenItselfTestsno longer claims a reservation cannot be given back: the replay cache can release one, and this path simply never does.