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

Update arg matcher misuse docs #549

Merged
merged 1 commit into from
Apr 23, 2019
Merged
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
66 changes: 57 additions & 9 deletions docs/help/_posts/2010-04-01-argument-matchers.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ public interface IFormatter {
string Format(object o);
}
public interface IWidgetFactory {
string Make(int i);
string Make(WidgetInfo info);
}
public class WidgetInfo {
public string Name { get; set; }
public int Quantity { get; set; }
}
public class Sprocket {
IWidgetFactory wf;
public Sprocket(IWidgetFactory wf) { this.wf = wf; }
public void StartWithWidget(int i) { wf.Make(i); }
public void StartWithWidget(WidgetInfo info) { wf.Make(info); }
}
ICalculator calculator;
IFormatter formatter;
Expand Down Expand Up @@ -129,7 +133,16 @@ Occasionally argument matchers get used in ways that cause unexpected results fo

### Using matchers outside of stubbing or checking received calls

Argument matchers should only be used when setting return values or checking received calls. Using `Arg.Is` or `Arg.Any` without a call to `Returns(...)` or `Received()` can cause your tests to behave in unexpected ways.
Argument matchers should only be used when specifying calls for the purposes of setting return values, checking received calls, or configuring callbacks (for example: with `Returns`, `Received` or `When`). Using `Arg.Is` or `Arg.Any` in other situations can cause your tests to behave in unexpected ways.

Argument matchers should only be used for:

* Specifying a call when using `Returns` and `ReturnsForAnyArgs`
* Specifying a call within a `When` or `WhenForAnyArgs` block to configure a callback/call action
* Specifying a call to check with `Received`, `DidNotReceive` and `Received.InOrder`
* Configuring a callback with `Arg.Do` or `Arg.Invoke`

Using an argument matcher without one of these calls is most likely an error.

For example:

Expand All @@ -140,25 +153,60 @@ var widgetFactory = Substitute.For<IWidgetFactory>();
var subject = new Sprocket(widgetFactory);

// OK: Use arg matcher for a return value:
widgetFactory.Make(Arg.Is<int>(x => x > 10)).Returns(TestWidget);
widgetFactory.Make(Arg.Is<WidgetInfo>(x => x.Quantity > 10)).Returns(TestWidget);

/* ACT */

// NOT OK: arg matcher used with a real call:
// subject.StartWithWidget(Arg.Any<int>());
// subject.StartWithWidget(Arg.Any<WidgetInfo>());
// Use a real argument instead:
subject.StartWithWidget(4);
subject.StartWithWidget(new WidgetInfo { Name = "Test", Quantity = 4 });

/* ASSERT */

// OK: Use arg matcher to check a call was received:
widgetFactory.Received().Make(Arg.Is<int>(x => x > 0));
widgetFactory.Received().Make(Arg.Is<WidgetInfo>(x => x.Name == "Test"));
```

In this example it would be an error to use an argument matcher in the `ACT` part of this test. Even if we don't mind what specific argument we pass to our subject, `Arg.Any` is only for substitutes, and only for specifying a call while setting return values, checking received calls or for configuring callbacks; not for real calls.

(If you do want to indicate to readers that the precise argument used for a real call doesn't matter you could use a variable such as `var someWidget = new WidgetInfo(); subject.StartWithWidget(someWidget);` or similar. Just stay clear of argument matchers for this!)

Similarly, we should not use an arg matcher as a real value to return from a call (even a substituted one):

```csharp
var widgetFactory = Substitute.For<IWidgetFactory>();

// NOT OK: using an arg matcher as a value, not to specify a call:
// widgetFactory.Make(new WidgetInfo { Name = "Test" }).Returns(Arg.Any<string>());
// Instead use something like:
widgetFactory.Make(new WidgetInfo { Name = "Test" }).Returns("any widget");
```

In this example it would be an error to use an argument matcher in the `ACT` part of this test. Even if we don't mind what specific argument we pass to our subject, `Arg.Any` is only for substitutes, and only for setting return values or checking received calls; not for real calls.
Another legal use of argument matchers is specifying calls when configuring callbacks:

```csharp
/* ARRANGE */
var widgetFactory = Substitute.For<IWidgetFactory>();
var subject = new Sprocket(widgetFactory);

// OK: Use arg matcher to configure a callback:
var testLog = new List<string>();
widgetFactory.When(x => x.Make(Arg.Any<WidgetInfo>())).Do(x => testLog.Add(x.Arg<WidgetInfo>().Name));

// OK: Use Arg.Do to configure a callback:
var testLog2 = new List<string>();
widgetFactory.Make(Arg.Do<WidgetInfo>(info => testLog2.Add(info.Name)));

/* ACT */
subject.StartWithWidget(new WidgetInfo { Name = "Test Widget" });

(If you do want to indicate to readers that the precise argument used for a real call doesn't matter you could use a variable such as `var someInt = 4; subject.StartWithWidget(someInt);` or similar. Just stay clear of argument matchers for this!)
/* ASSERT */
Assert.AreEqual(new[] { "Test Widget" }, testLog);
Assert.AreEqual(new[] { "Test Widget" }, testLog2);
```

### Modifying values being matched

Expand Down