90

I have a Transfer class, simplified it looks like this:

public class Transfer
{
    public virtual IFileConnection source { get; set; }
    public virtual IFileConnection destination { get; set; }

    public virtual void GetFile(IFileConnection connection, 
        string remoteFilename, string localFilename)
    {
        connection.Get(remoteFilename, localFilename);
    }

    public virtual void PutFile(IFileConnection connection, 
        string localFilename, string remoteFilename)
    {
        connection.Get(remoteFilename, localFilename);
    }

    public virtual void TransferFiles(string sourceName, string destName)
    {
        source = internalConfig.GetFileConnection("source");
        destination = internalConfig.GetFileConnection("destination");
        var tempName = Path.GetTempFileName();
        GetFile(source, sourceName, tempName);
        PutFile(destination, tempName, destName);
    }
}

The simplified version of the IFileConnection interface looks like this:

public interface IFileConnection
{
    void Get(string remoteFileName, string localFileName);
    void Put(string localFileName, string remoteFileName);
}

The real class is supposed to handle a System.IO.IOException that is thrown when the IFileConnection concrete classes loses connectivity with the remote, sending out emails and what not.

I would like to use Moq to create a Transfer class, and use it as my concrete Transfer class in all properties and methods, except when the GetFile method is invoked - then I want it to throw a System.IO.IOException and make sure the Transfer class handles it properly.

Am I using the right tool for the job? Am I going about this the right way? And how would I write the setup for that unit test for NUnit?

SteveC
  • 15,808
  • 23
  • 102
  • 173
Jeremy Holovacs
  • 22,480
  • 33
  • 117
  • 254
  • Basically, this is the wrong approach - you should be mocking the IFileConnection, and newing up a Transfer object in your test, and passing the stubs in - they're not mocks! Depending on the permutation of behaviour of those mocks, your test will cover different parts of the code. Look at code coverage in something like Rider or Visual Studio – kenno Oct 18 '19 at 05:16
  • No, they're not mocks, they are the actual objects with a single property mocked... that was the point of the question. I think maybe you didn't understand what the ask was here? – Jeremy Holovacs Oct 23 '19 at 04:12

3 Answers3

120

Here's how you can mock your FileConnection

Mock<IFileConnection> fileConnection = new Mock<IFileConnection>(
                                                           MockBehavior.Strict);
fileConnection.Setup(item => item.Get(It.IsAny<string>,It.IsAny<string>))
              .Throws(new IOException());

Then instantiate your Transfer class and use the mock in your method call

Transfer transfer = new Transfer();
transfer.GetFile(fileConnection.Object, someRemoteFilename, someLocalFileName);

Update:

First of all you have to mock your dependencies only, not the class you are testing(Transfer class in this case). Stating those dependencies in your constructor make it easy to see what services your class needs to work. It also makes it possible to replace them with fakes when you are writing your unit tests. At the moment it's impossible to replace those properties with fakes.

Since you are setting those properties using another dependency, I would write it like this:

public class Transfer
{
    public Transfer(IInternalConfig internalConfig)
    {
        source = internalConfig.GetFileConnection("source");
        destination = internalConfig.GetFileConnection("destination");
    }

    //you should consider making these private or protected fields
    public virtual IFileConnection source { get; set; }
    public virtual IFileConnection destination { get; set; }

    public virtual void GetFile(IFileConnection connection, 
        string remoteFilename, string localFilename)
    {
        connection.Get(remoteFilename, localFilename);
    }

    public virtual void PutFile(IFileConnection connection, 
        string localFilename, string remoteFilename)
    {
        connection.Get(remoteFilename, localFilename);
    }

    public virtual void TransferFiles(string sourceName, string destName)
    {
        var tempName = Path.GetTempFileName();
        GetFile(source, sourceName, tempName);
        PutFile(destination, tempName, destName);
    }
}

This way you can mock internalConfig and make it return IFileConnection mocks that does what you want.

SteveC
  • 15,808
  • 23
  • 102
  • 173
Ufuk Hacıoğulları
  • 37,978
  • 12
  • 114
  • 156
  • Hmm. Not what I was expecting, and now I see I simplified my `Transfer` class too much. Can you look at the changes I made to the `Transfer` class? This resembles my code a bit more closely. The properties are exposed in the class, but I did that mainly so I could test the exception condition. Is what I was trying to do the wrong way? – Jeremy Holovacs Apr 25 '12 at 22:16
  • Hmm. What I thought would be simple seems to be getting less so. Let me ask you this: would it make more sense to create a test class that inherits from `Transfer` and override the method for testing purposes? Perhaps mocking is not the proper technology to use in this scenario, and I'm trying to shove a square peg into a round hole? – Jeremy Holovacs Apr 25 '12 at 23:34
  • 1
    @JeremyHolovacs No, I don't think that would be a good idea. Will you create subclasses for each scenario instead of making your design more testable? You have to use the class under test as is, mock other objects it depends on. This way you can be sure that the class under test is working correctly which is the purpose of unit testing. – Ufuk Hacıoğulları Apr 26 '12 at 06:47
19

I think this is what you want, I already tested this code and works

The tools used are: (all these tools can be downloaded as Nuget packages)

http://fluentassertions.codeplex.com/

http://autofixture.codeplex.com/

http://code.google.com/p/moq/

https://nuget.org/packages/AutoFixture.AutoMoq

var fixture = new Fixture().Customize(new AutoMoqCustomization());
var myInterface = fixture.Freeze<Mock<IFileConnection>>();

var sut = fixture.CreateAnonymous<Transfer>();

myInterface.Setup(x => x.Get(It.IsAny<string>(), It.IsAny<string>()))
        .Throws<System.IO.IOException>();

sut.Invoking(x => 
        x.TransferFiles(
            myInterface.Object, 
            It.IsAny<string>(), 
            It.IsAny<string>()
        ))
        .ShouldThrow<System.IO.IOException>();

Edited:

Let me explain:

When you write a test, you must know exactly what you want to test, this is called: "subject under test (SUT)", if my understanding is correctly, in this case your SUT is: Transfer

So with this in mind, you should not mock your SUT, if you substitute your SUT, then you wouldn't be actually testing the real code

When your SUT has external dependencies (very common) then you need to substitute them in order to test in isolation your SUT. When I say substitute I'm referring to use a mock, dummy, mock, etc depending on your needs

In this case your external dependency is IFileConnection so you need to create mock for this dependency and configure it to throw the exception, then just call your SUT real method and assert your method handles the exception as expected

  • var fixture = new Fixture().Customize(new AutoMoqCustomization());: This linie initializes a new Fixture object (Autofixture library), this object is used to create SUT's without having to explicitly have to worry about the constructor parameters, since they are created automatically or mocked, in this case using Moq

  • var myInterface = fixture.Freeze<Mock<IFileConnection>>();: This freezes the IFileConnection dependency. Freeze means that Autofixture will use always this dependency when asked, like a singleton for simplicity. But the interesting part is that we are creating a Mock of this dependency, you can use all the Moq methods, since this is a simple Moq object

  • var sut = fixture.CreateAnonymous<Transfer>();: Here AutoFixture is creating the SUT for us

  • myInterface.Setup(x => x.Get(It.IsAny<string>(), It.IsAny<string>())).Throws<System.IO.IOException>(); Here you are configuring the dependency to throw an exception whenever the Get method is called, the rest of the methods from this interface are not being configured, therefore if you try to access them you will get an unexpected exception

  • sut.Invoking(x => x.TransferFiles(myInterface.Object, It.IsAny<string>(), It.IsAny<string>())).ShouldThrow<System.IO.IOException>();: And finally, the time to test your SUT, this line uses the FluenAssertions library, and it just calls the TransferFiles real method from the SUT and as parameters it receives the mocked IFileConnection so whenever you call the IFileConnection.Get in the normal flow of your SUT TransferFiles method, the mocked object will be invoking throwing the configured exception and this is the time to assert that your SUT is handling correctly the exception, in this case, I am just assuring that the exception was thrown by using the ShouldThrow<System.IO.IOException>() (from the FluentAssertions library)

References recommended:

http://martinfowler.com/articles/mocksArentStubs.html

http://misko.hevery.com/code-reviewers-guide/

http://misko.hevery.com/presentations/

http://www.youtube.com/watch?v=wEhu57pih5w&feature=player_embedded

http://www.youtube.com/watch?v=RlfLCWKxHJ0&feature=player_embedded

Dima
  • 1,717
  • 15
  • 34
Jupaol
  • 21,107
  • 8
  • 68
  • 100
  • I don't think this does what I'd need it to do. I need to throw the exception in the middle of the `Transfer.TransferFiles()` method by letting the `Transfer.TransferFiles()` method call invoke the exception in the `GetFile()` method. I think (if I'm reading this right) this injects the `IFileConnection` at runtime, which would get written over. Am I wrong? – Jeremy Holovacs Apr 26 '12 at 13:48
  • you forgot the parentheses after Should `.ShouldThrow();` should be `.Should().Throw();` – Hamza Aug 29 '21 at 17:25
12

This is how I managed to do what I was trying to do:

[Test]
public void TransferHandlesDisconnect()
{
    // ... set up config here
    var methodTester = new Mock<Transfer>(configInfo);
    methodTester.CallBase = true;
    methodTester
        .Setup(m => 
            m.GetFile(
                It.IsAny<IFileConnection>(), 
                It.IsAny<string>(), 
                It.IsAny<string>()
            ))
        .Throws<System.IO.IOException>();

    methodTester.Object.TransferFiles("foo1", "foo2");
    Assert.IsTrue(methodTester.Object.Status == TransferStatus.TransferInterrupted);
}

If there is a problem with this method, I would like to know; the other answers suggest I am doing this wrong, but this was exactly what I was trying to do.

Dima
  • 1,717
  • 15
  • 34
Jeremy Holovacs
  • 22,480
  • 33
  • 117
  • 254
  • No, this is accepted answer is completely wrong! You're asserting on a Mock.Object - only testing your Mock!!! The Transfer object should not be mocked, only its FileConnection dependencies. – kenno Oct 18 '19 at 05:13
  • No, it's actually testing the underlying object (see the CallBase property.) – Jeremy Holovacs Oct 23 '19 at 04:09