Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make conditionalSetup.Verifiable() an error #997

Merged
merged 1 commit into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1

* Two new public methods in `Times`: `bool Validate(int count)` and `string ToString()` (@stakx, 975)

#### Changed

* Attempts to mark conditionals setup as verifiable are now considered an error, since conditional setups are ignored during verification. Calls to `.Verifiable()` on conditional setups are no-ops and can be safely removed. (@stakx, #997)

#### Fixed

* Regression: Restored `Capture.In` use in `mock.Verify(expression, ...)` to extract arguments of previously recorded invocations. (@vgriph, #968; @stakx, #974)
Expand Down
9 changes: 9 additions & 0 deletions src/Moq/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Moq/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,7 @@ This could be caused by an unrecognized type conversion, coercion, narrowing, or
<data name="UseItIsOtherOverload" xml:space="preserve">
<value>It is impossible to call the provided strongly-typed predicate due to the use of a type matcher. Provide a weakly-typed predicate with two parameters (object, Type) instead.</value>
</data>
<data name="ConditionalSetupsAreNotVerifiable" xml:space="preserve">
<value>This call to 'Verifiable' will have no effect because conditional setups are ignored by both 'Verify' and 'VerifyAll'. This might indicate an error in your code.</value>
</data>
</root>
7 changes: 7 additions & 0 deletions src/Moq/Setup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Reflection;
using System.Text;

using Moq.Properties;

namespace Moq
{
internal abstract class Setup : ISetup
Expand Down Expand Up @@ -86,6 +88,11 @@ public void MarkAsOverridden()

public void MarkAsVerifiable()
{
if (this.IsConditional)
{
throw new InvalidOperationException(Resources.ConditionalSetupsAreNotVerifiable);
}

this.flags |= Flags.Verifiable;
}

Expand Down
15 changes: 7 additions & 8 deletions tests/Moq.Tests/VerifyFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1047,17 +1047,16 @@ public void CanVerifyMethodThatIsNamedLikeEventRemoveAccessor()
}

[Fact]
public void Verify_ignores_conditional_setups()
public void Verify_ignores_conditional_setups_so_calling_Verifiable_is_a_user_error()
{
var mock = new Mock<IFoo>();
mock.When(() => true).Setup(m => m.Submit()).Verifiable();
var setup = mock.When(() => true).Setup(m => m.Submit());

var exception = Record.Exception(() =>
{
mock.Verify();
});

Assert.Null(exception);
// Conditional setups are completely ignored by `Verify[All]`.
// Making such a setup verifiable is therefore a no-op, but
// may be indicative of the user making an incorrect assumption;
// best to warn the user so they revise their code:
Assert.Throws<InvalidOperationException>(() => setup.Verifiable());
}

[Fact]
Expand Down