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

Allow specifying action if setup was not met in strict mode #856

Closed
patchmeifyoucan opened this issue Jul 5, 2019 · 2 comments
Closed

Comments

@patchmeifyoucan
Copy link

Let's say I try to set up a Mock to behave like a database. For this, I have a simple list with three strings representing identifiers. Then I have following setup:

mock
.Setup(m => m.Action(It.IsIn(idsList)))
.ReturnsAsync((string s) => objectList.First(o => o.Id == s));

mock
.Setup(m => m.Action(It.IsNotIn(idsList)))
.Throws<NotFoundException>();

There are many methods that operate on identifiers. As in a real world scenario, supplying an ID that does not exist leads to some (hopefully) predictable error. I dislike my setup because:

  • it is verbose, for every method that has behavior like this, setup has to be done twice
  • every condition has to be inverted, this is not the big deal since they are logically trivial but it's still cumbersome to write those setups if there are many
  • if the exact setup is not met, strict mode will throw a MockException

Even though one could get around the verbose setup with some additional C# code by extending Linq, it is not possible to do so to avoid the MockException. Switching to loose mode is not an option because strict mode is great.

But for the scenario described above it would be nice to be able to do something like this:

mock
.SetupIf(m => m.Action(It.IsIn(idsList)))
.ReturnsAsync((string s) => objectList.First(o => o.Id == s))
.ElseAsync((params go here) => { custom action if setup is not met });

This would allow to act powerful on these conditions. Since I don't every method to behave like this, the "normal" setup should rather stay as it is.

Since one will mostly throw specialized Exceptions in these cases, syntax sugar would be also appreciated, i.e.:

mock
.SetupIf(m => m.Action(It.IsIn(idsList)))
.ReturnsAsync((string s) => objectList.First(o => o.Id == s))
.ElseThrowsAsync<NotFoundException>();

I found this piece of code that is responsible for the current behavior:

    /// <summary>
    ///   Returns the exception to be thrown when a strict mock has no setup corresponding to the specified invocation.
    /// </summary>
    internal static MockException NoSetup(Invocation invocation)
    {
      return new MockException(MockExceptionReasons.NoSetup, string.Format((IFormatProvider) CultureInfo.CurrentCulture, Resources.MockExceptionMessage, (object) invocation.ToString(), (object) MockBehavior.Strict, (object) Resources.NoSetup));
    }

Could this somehow be modified to pick up a provided error as described above?

@stakx
Copy link
Contributor

stakx commented Jul 7, 2019

I don't think it would be a good idea to make strict mocks' by-default throwing behavior configurable, because that would make the meaning of "strict mock" less clear. Having a strict mock means that you need to provide the behavior (i.e. as setups) for each and every expected call. As soon as you introduce some kind of user-defined default behavior, this rule no longer holds and "strict" becomes a little less meaningful.

Therefore, perhaps the right place for a fallback behavior is not at the level of the mock, but of an individual setup.

mock
.SetupIf(m => m.Action(It.IsIn(idsList)))
.ReturnsAsync((string s) => objectList.First(o => o.Id == s))
.ElseThrowsAsync<NotFoundException>();

There's a problem with the if-else syntax you propose. Either this setup is matched, and it gets executed, or it isn't matched, and its "else" part gets executed. So what if there are other setups that would match? Should the "else" part still be executed, or should another, better-matching setup be preferred over this one?

We don't need to find an answer to these questions because Moq already gives you the means to specify a fallback. Later setups generally "win" over earlier ones, so you can do this:

mock.Setup(m => m.Action(It.IsAny<string>()).Throws(new NotFoundException());
mock.Setup(m => m.Action(It.IsIn(idsList)).Returns((string s) => objectList.First(o => o.Id == s));

(Or, if you don't want two setups, combine them into one:)

mock.Setup(m => m.GetById(It.IsAny<string>()))
    .Returns((string id) => objectList.FirstOrDefault(o => o.Id == id) ?? throw new NotFoundException());

No special if-else setup API required. And, if you do that repeatedly and want to stay DRY, you can create a helper method for this setup pattern.

IIRC, the upcoming Moq v5 might allow you to configure the default mock behavior more flexibly than is possible in v4.

@patchmeifyoucan
Copy link
Author

Good answer. My intention was to not have any "logic" in mocks but a null check should be fine. It's a good hint that default behavior independent of values can be set up like that. Indeed, defining the negative case first is a shorter and more readable after one gets used to the idea.

Thanks for the reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants