Skip to content

Doc fixes #606

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

Merged
merged 1 commit into from
Mar 4, 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
2 changes: 1 addition & 1 deletion docs/help/_posts/2010-01-01-getting-started.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ We can ask NSubstitute to create a substitute instance for this type. We could a
calculator = Substitute.For<ICalculator>();
```

⚠️ **Note**: NSubstitute will only work properly with interfaces or with class members that are overridable from the test assembly (`public virtual`, `protected virtual`, `protected internal virtual`, or `internal virtual` with `InternalsVisibleTo` attribute applied). Be careful substituting for classes with non-virtual members. See [Creating a substitute](/help/creating-a-substitute/#substituting_infrequently_and_carefully_for_classes) and [How NSubstitute works](/help/how-nsub-works) for more information.
⚠️ **Note**: NSubstitute will only work properly with interfaces, or with class members that are overridable from the test assembly. Be very careful substituting for classes with non-virtual or `internal virtual` members, as real code could be inadvertently executed in your test. See [Creating a substitute](/help/creating-a-substitute/#substituting_infrequently_and_carefully_for_classes) and [How NSubstitute works](/help/how-nsub-works) for more information. Also make sure to install [NSubstitute.Analyzers](/help/nsubstitute-analysers) which can warn about many of these cases (but not all of them; be careful with classes!).

Now we can tell our substitute to return a value for a call:

Expand Down
2 changes: 1 addition & 1 deletion docs/help/_posts/2010-01-02-creating-a-substitute.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ This is how you'll normally create substitutes for types. Generally this type wi

⚠️ **Warning:** Substituting for classes can have some nasty side-effects!

For starters, **NSubstitute can only work with *virtual* members of the class** that are overridable in the test assembly, so any non-virtual code in the class will actually execute! If you try to substitute for a class that formats your hard drive in the constructor or in a non-virtual property setter then you're asking for trouble. (By overridable we mean `public virtual`, `protected virtual`, `protected internal virtual`, or `internal virtual` with `InternalsVisibleTo` attribute applied. See [How NSubstitute works](/help/how-nsub-works) for more information.)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still has protected virtual in the doc. I thought we wanted to remove it. Am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit this is confusing. These docs are referring to real code executing for non-overridable members, which includes protected virtual code, even if these calls are not (easily) configurable via the NSubstitute API. It's not about what methods we can configure with NSub -- it's about what real code can execute.

There is probably a better way to communicate this. 🤔

For starters, **NSubstitute can only work with *virtual* members of the class** that are overridable in the test assembly, so any non-virtual code in the class will actually execute! If you try to substitute for a class that formats your hard drive in the constructor or in a non-virtual property setter then you're asking for trouble. By overridable we mean `public virtual`, `protected virtual`, `protected internal virtual`, or `internal virtual` with `InternalsVisibleTo` attribute applied (although to configure or assert on calls members will also need to be callable from the test assembly, so `public virtual` or `internal virtual` with `InternalsVisibleTo`). See [How NSubstitute works](/help/how-nsub-works) for more information.

It also means features like `Received()`, `Returns()`, `Arg.Is()`, `Arg.Any()` and `When()..Do()` **will not work with these non-overridable members**. For example: `subClass.Received().NonVirtualCall()` will not actually run an assertion (it will always pass, even if there are no calls to `NonVirtualCall()`), and can even cause confusing problems with later tests. These features will work correctly with virtual members of the class, but we have to be careful to avoid the non-virtual ones.

Expand Down
6 changes: 3 additions & 3 deletions docs/help/_posts/2015-01-01-how-nsub-works.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ There are some caveats when `Original` is a class though (hence all the [warning

If `DoStuffWith(string s)` is not `virtual`, the `SubstituteForOriginal` class will not be able to override it, so when it is called NSubstitute will not know about it. It is effectively invisible to NSubstitute; it can't record calls to it, it can't configure values using `Returns`, it can't run actions via `When..Do`, it can't verify the call was received. Instead, the real base implementation of the member will run.

This can cause all sorts of problems if we accidentally attempt to configure a non-virtual call, because NSubstitute will get confused about which call you're talking about. Usually this will result in a run-time error, but in the worst case it can affect the outcome of your test, or even the following test in the suite, in non-obvious ways. Thankfully we have [NSubstitute.Analyzers](/help/nsubstitute-analyzers) to detect these cases at compile time.
This can cause all sorts of problems if we accidentally attempt to configure a non-virtual call, because NSubstitute will get confused about which call you're talking about. Usually this will result in a run-time error, but in the worst case it can affect the outcome of your test, or even the following test in the suite, in non-obvious ways. Thankfully we have [NSubstitute.Analyzers](/help/nsubstitute-analysers) to detect these cases at compile time.

### Internal members

Expand All @@ -55,7 +55,7 @@ Similar limitations apply to `internal virtual` members. Because `SubstituteForO

Remember that if the member is non-virtual, NSubstitute will not be able to intercept it regardless of whether it is `internal` or `InternalsVisibleTo` has been added.

The good news is that [NSubstitute.Analyzers](/help/nsubstitute-analyzers) will also detect attempts to use `internal` members at compile time, and will suggest fixes for these cases.
The good news is that [NSubstitute.Analyzers](/help/nsubstitute-analysers) will also detect attempts to use `internal` members at compile time, and will suggest fixes for these cases.

### Real code

Expand All @@ -66,6 +66,6 @@ The final thing to notice here is that there is the potential for real logic fro
* Be careful substituting for classes!
* Where possible use interfaces instead.
* Remember NSubstitute works by inheriting from (or implementing) your original type. If you can't override a member by manually writing a sub-class, then NSubstitute won't be able to either!
* Install [NSubstitute.Analyzers](/help/nsubstitute-analyzers) where ever you install NSubstitute. This will help you avoid these (and other) pitfalls.
* Install [NSubstitute.Analyzers](/help/nsubstitute-analysers) where ever you install NSubstitute. This will help you avoid these (and other) pitfalls.