This repository was archived by the owner on Jun 30, 2023. It is now read-only.
merge initial legacy support into master #40
Merged
Conversation
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
Collaborator
adalon
commented
Jul 25, 2019
adalon
commented
- Legacy API shim
- CallBase support
- CodeFixer/CodeGen improvements
Add support for v4 APIs implemented in terms of the new API, in non-browsable mode so that new code can leverage the new idioms instead. Removes the `Sdk.Times` struct from Moq's public `Verify` APIs to avoid conflicts with the legacy `Times`, since it is not necessary given the fluent occurrence constraints methods (`Once`, `Never`, `AtLeastOnce` and `Exactly`).
Annotating the constructor methods and modifying the source for the type arguments is enough for now.
This ensures that the underlying pipeline is registered right when `mock.Setup(..)` is called, which would in turn eventually setup default pipeline behaviors as needed (i.e. for the upcoming `CallBase`).
When creating the legacy Mock<T> we need to make sure to use the "external" calling assembly when passing arguments between the ctor overloads.
This allows extending the execution pipeline by dynamically specifying that certain behaviors (by type) should be excluded from execution for the current invocation. This will allow adding `CallBase` support for example, by allowing StrictBehavior to skip throwing and let the base implementation in a base class to run.
When the pipeline is run in a `SetupScope`, we should not run the entire pipeline, but rather just the behaviors we know are safe for execution during setup, specifically `MockContextBehavior` (to set up the `MockContext.CurrentInvocation` and `MockContext.CurrentSetup`) and `DefaultValueBehavior` to return a default value for further fluent API invocations to work on. This simplifies the behaviors since they don't need to check and bail during active setup scopes, and makes them more self-contained.
This allows simplified lambdas for setup, so that ``` mock.Setup(x => x.TurnOn()); ``` becomes ``` mock.Setup(() => mock.TurnOn()); ``` A future version could also add `Setup(() ...)` to Syntax for the same effect.
Instead of duplicating behavior and having the Behavior property modify the actual pipeline, leverage the new mechanism we added to allow skipping behaviors dynamically for the current invocation. With this new feature at the pipeline level, we can create a new configuration behavior that can check the `MockBehavior` and simply skip the `StrictMockBehavior` if it's not `MockBehavior.Strict`. This greatly simplifies the implementation and makes both default and strict behaviors always present on the pipeline, but dynamically skipped based on configuration. This requires introducing a mock state-level "field", which we now save with the key `IMoq.Behavior`, which should be the guideline for extensions that need to persist state in the mock: use the `[Type].[Property]` syntax to minimize chances of collision.
This reverts commit 0c2ff79. That commit broke the analyzer for ref/out delegate generation since the new overloads would match some of those setups. We need to think better how that will work in the future, since overload resolution in the compiler will not distinguish between two signatures that differ only in parameter mode (ref/out).
The behaviors are actually the ones initialized by `MockInitializer` so it makes sense that the one that sets which ones to skip also lives there instead of the Moq.Sdk.
We don't want to accidentally pollute the public visible API.
Since it's already part of the default processors but we're adding it again after we run the Mock-specific processors.
The NuGet asset resolving target already adds the metadata to the `Analyzer` items inferred from the `analyzer` folder in the package, but the `CodeFix` we provided did not contain this metadata. This makes it harder for a consumer to actually filter/remove them if they want to customize codegen in advanced way. This would also allow a project to replace the package-provided analyzers and code fixers with locally built ones directly from the bin directories of the Moq solution.
Based on the ASP.NET Core solution, we added the following optimizations: * Add a FixAll provider to support batch code fixing of all mocks/stunts that need codegen * Do not get *all* diagnostics from *all* analyzers when getting code fixes, but only those from analyzers that produce diagnostics the desired code fix provider can actually fix (otherwise, we were also getting a ton of diagnostics that might have nothing to do with the code fix provider and intended code fix name) This *significantly* improves the performance of the code generation with real world solutions.
This turned out to consume an inordinate amount of time for the benefit it provides, so we don't do it anymore. We still do the namespace sorting since it's cheaper.
marianor
reviewed
Jul 25, 2019
| // Once with message | ||
| Assert.Throws<VerifyException>(() => calculator.Verify(x => x.TurnOn(), 1, "Should have been called!")); | ||
| // Times.Once with message | ||
| Assert.Throws<VerifyException>(() => calculator.Verify(x => x.TurnOn(), Times.Once, "Should have been called!")); |
There was a problem hiding this comment.
For legacy support Once should be a method
Member
There was a problem hiding this comment.
Good point, but it never made sense as a method in the first place, since it has no parameters :(.
The good thing is that this is the new API overload. The old API overloads could still provide backwards compatible members, I think.
marianor
approved these changes
Jul 25, 2019
marianor
approved these changes
Jul 25, 2019
Adds support for mocking generic types, which wasn't working before.
We previously used the very rudimentary `MetadataName` property to determine the type to generate a stunt for. This was error prone since it does not contain the full metadata for generic types (or nullable types) and basically you cannot resolve an `INamedTypeSymbol` from it. Since we need to support a more reliable and robust way to communicate back from an analyzer to its code fix what is the target type(s) to generate stunts for, we implement our own formatting (leveraging the `SymbolDisplayFormat` and then implementing a resolver that parses the C#-like generic expression (i.e. `IEnumerable<KeyValuePair<string, int?>>`) and resolves it back to a fully constructed generic `INamedTypeSymbol`. We leverage the Superpower library for the parsing which is fast, provides great error reporting and is a breeze to use and understand.
This code fix should only load in our own workspace, since csc does not contain the required dependencies for it to load properly. This was causing transient build errors when csc.exe would try to inspect the types via MEF and fail in the static initialization of the RoslynInternals class which even if not used since the code fix was not exported, was nevertheless initialized statically when reflected by the MEF composition. By moving it to the CodeFix assembly, we also make it work transparently like all other code fixers, which are referenced just by name. We were already doing that from the C# and VB scaffold document processors, so no changes were necessary there.
Leave the generated class visibility up to the consumer, who can declare a partial class and make it public that way. This ensures we can successfully mock internals which have IVT with the test project.
By moving the code to its own class with its own state, we avoid using lambda state capturing which could be problematic since we're running all of the fixers in parallel to boost performance.
At some point these tests were not working, but as part of testing the new relocated override-all code fix, we found out they actually do work now again :). Made the dynamic stunt disposable to isolate better from one test to the next anyway, which should also make them more resilient by disposing the created ad-hoc workspace.
Code generation support for generics introduced the `Of..` name to append the generic types being mocked (i.e. `IEnumerableOfStringMock`). This was not reflected in the run-time naming classes so some tests were failing. In addition, there was quite some duplication between MockNaming and StuntNaming, which is now unified with the former delegating to the latter all behavior and just providing different DefaultNamespace and DefaultSuffix values. The new implementation is more aligned with the NamingConvention used for codegen now.
Instead of the quite complicated implementation that tried replacing the pipeline on a mock, instantiating a new one, cloning its state, etc., we will just provide an analyzer+codefix that will expose the `T` as part of the generic `new Mock<T...>` constructor, which means the code generator doesn't need to get any smarter, and we can actually help migrate people to the newer pattern where all types are known to the factory from the beginning. In order to do this, we also brought back the constraint on the `T` for `IMock<T>`, which aligns with v4 and also allows us to cast from within the `As<T>` implementation. This constraint was added to all APIs that exposed `IMock<T>`.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.