1

I have created a unit test method that verifies method is called, below is the code for the same.The method builds email object and calls GeneratePDF method which returns bytes further BuildEmailInfo method returns email object.

public class SMTPEmailSender : IEmailSender
{
    private IPDFCreater _pdfCreater;

    public SMTPEmailSender(IPDFCreater pdfCreater)
    {
        _pdfCreater = pdfCreater;
    }
    public Email BuildEmailInfo(string sMTPServerUrl, FaxMailDTO faxAsMailRequest)
    {
        Email email=null;
        try
        {
            var otp = new PDFData { OTP =faxAsMailRequest.OTP};
            email = new Email
            {
                SMTPServerUrl = sMTPServerUrl,
                Encoding = Encoding.UTF8,

                ToAddress = faxAsMailRequest.ToEmailAddress,
                ToAddressDisplayName = faxAsMailRequest.ToAddressDisplayName,
                FromAddress = faxAsMailRequest.FromEmailAddress,
                Subject = faxAsMailRequest.Subject,
                Body = faxAsMailRequest.Body,
                FromAddressDisplayName = faxAsMailRequest.FromAddressDisplayName,
                ContentStream = new MemoryStream(_pdfCreater.GeneratePDF(otp)),
                AttachmentName = faxAsMailRequest.FaxFileName
                
            };
        }
        catch(Exception ex)
        {
            Log.Error("Method : BuildEmailInfo. Exception raised while building email data : {@Message}", ex.Message, ex);
        }
        

       

        return email;
    }

Below is my unit test code , whenever I execute this it throws an error Expected invocation on the mock at least once, but was never performed: x=>x.GeneratePDF(pdfdata). Also Let me know if it is right way to perform the test

public class SMTPEmailSenderTest
{
    private SMTPEmailSender _sMTPEmailSender;
    Mock<IPDFCreater> _mockPdfCreator;
    public SMTPEmailSenderTest()
    {
        _mockPdfCreator = new Mock<IPDFCreater>();
        _sMTPEmailSender = new SMTPEmailSender(_mockPdfCreator.Object);
    }


    [Theory]
    [MemberData(nameof(GetFaxAsMailObject))]
    public void BuildEmailInfoTest_ReturnsValidEmailObject(FaxMailDTO faxMailDTO)
    {
        string smpturl = "localhost";
        var otp = new PDFData { OTP = faxMailDTO.OTP };
        var result = _sMTPEmailSender.BuildEmailInfo(smpturl, faxMailDTO);
        _mockPdfCreator.Verify(x => x.GeneratePDF(otp));
    }
  }
Vishal Dhasal
  • 51
  • 1
  • 10
  • I know quetzalcoatl already found your main issue. But it also appears to me that an exception will always be thrown in BuildMailInfo unless you do a mock setup. Otherwise the constructor for MemoryStream will always get a null argument. – John Pankowicz Oct 10 '20 at 15:44

2 Answers2

3

This line:

        _mockPdfCreator.Verify(x => x.GeneratePDF(otp));

performs a 'verification'. This is an assertion that checks if method .GeneratePDF has been called on _mockPdfCreator with otp as its parameter.

All .Verify methods from the interface of the Mock object are used to check if some method or property were called. You can also provide some filters to see if certain parameters were passed, for example:

        _myMock.Verify(x => x.FooBar(5));
        _myMock.Verify(x => x.FooBar(123));
        _myMock.Verify(x => x.FooBar(It.IsAny<int>());
        _myMock.Verify(x => x.FooBar(It.Is<int>(number => (number-5)%3 > 10));

all of these check if FooBar was alled on _myMock, but each of them looks only at calls that used certain values of parameters: 5, 123, anything-that-is-int, or (...).

You cannot use .Verify to check for the return value.
There's no such option there.

Why? Think about it. You've had:

    _mockPdfCreator = new Mock<IPDFCreater>();
    ....
    _mockPdfCreator.Verify(x => x.GeneratePDF(otp));

The _mockPdfCreator is your mock object. Not a real thing. It's a tiny ghost that acts as if it were some IPDFCreater.

There's not a slightest bit of a real implementation there.
How can you expect that GeneratePDF returns anythin meaningful?
It just won't. Nothing's there behind it. If anything called that method GeneratePDF, it would return NULL (or throw exception, depending on mocking mode: Loose/Strict).

...unless you SET UP your mock to do it differently:

    var theThing = ...;
    _mockPdfCreator = new Mock<IPDFCreater>();
    _mockPdfCreator.Setup(x => x.GeneratePDF(It.IsAny<...>())).Returns(theThing);
    ....
    // ... now check what `GeneratePDF` returned?!

Now of anything calls GeneratePDF method, it will return theThing. Alright.
But you knew that already. There's nothing to check. You set up GeneratePDF to return the thing, so there's not a slightest point to check what GeneratePDF returned. It's your mock and your setup!


Sooo, if anything called GeneratePDF, then NULL would be returned, because there's no setup for GeneratePDF. However, as Verify proved, the GeneratePDF was never called. This means that when you created the SMTPEmailSender giving it the mock as parameter:

    _mockPdfCreator = new Mock<IPDFCreater>();
    _sMTPEmailSender = new SMTPEmailSender(_mockPdfCreator.Object);

and then in test you've had:

    ....
    var result = _sMTPEmailSender.BuildEmailInfo(smpturl, faxMailDTO);
    _mockPdfCreator.Verify(x => x.GeneratePDF(otp));

then, apparently, the _sMTPEmailSender.BuildEmailInfo didn't fancy calling GeneratePDF at all.

Why? No idea. Most probably there was something either in smpturl or faxMailDTO that was considered invalid for this use case and that generate-pdf step was skipped. Check the Result. See if there are any errors or messages that would tell you anything about why it did not even try to call GeneratePDF.

Also note that the verification you wrote were

x => x.GeneratePDF(otp)

That's pretty specific. It has hard-coded reference to otp. So maybe it was called, but with different parameter value?

Try adding:

    var result = _sMTPEmailSender.BuildEmailInfo(smpturl, faxMailDTO);
    _mockPdfCreator.Verify(x => x.GeneratePDF(It.IsAny<PDFData>())); // <-
    _mockPdfCreator.Verify(x => x.GeneratePDF(otp));

or something along the lines and see which Verify fails. If the former passes and the latter fails, than everything's mostly fine, it's just not the exact OTP that you expected (maybe _sMTPEmailSender cloned it? etc).

In any chance that the former fails, then it means that GeneratePDF is truly not called even a single time, and then it means you have to learn why BuildEmailInfo with params (smpturl, faxMailDTO) doesn't do what you expect. You've got a try-catch-log there. Maybe some nullreferenceexption? But I doubt it.

You've got there:

[MemberData(nameof(GetFaxAsMailObject))]  /// <==== B
public void BuildEmailInfoTest_ReturnsValidEmailObject(FaxMailDTO faxMailDTO) // <--- A
{
    ...
    var otp = new PDFData { OTP = faxMailDTO.OTP }; //<--- C
    ...
    _mockPdfCreator.Verify(x => x.GeneratePDF(otp)); //<---D

So, the faxMailDTO is from GetFaxAsMailObject. The BuildEmailInfo gets it via params and passes part of it to GeneratePDF. Then you assert in Verify that D uses newly-constructed otp from line C. That just can't work. The faxMailDTO from A+B so from GetFaxAsMailObject certainly DOES NOT contain otp from C and certainly will not pass otp object to GeneratePDF. The GeneratePDF will get some other PDFData object that came from faxMailDTO from A+B.

I think I've said enough and covered all issues with your test setup.. You almost have it right. Good luck!

quetzalcoatl
  • 32,194
  • 8
  • 68
  • 107
0

Verifications on mock objects should be your 'last resort' in unit tests. Think about it: is an actual requirement violated if the PDF creator does not call the GeneratePDF method? The user only cares that the PDF was generated.

In this case, you can verify the outcome of the BuildEmailInfo method directly, for example:

var result =  _sMTPEmailSender.BuildEmailInfo(smpturl, faxMailDTO);
var expectedBytes = ...; // TODO - get the expected byte[] array from somewhere
Assert.Equal(expectedBytes, result.ContentStream.ToArray());

Additionally, you may be able to write this test without mocking the dependency at all? If the actual PDF creator object can be called to generate a byte[] array in memory, you could just use the real object instead of mocking it.

jeroenh
  • 26,362
  • 10
  • 73
  • 104
  • How could we use real object , can you please guide me on this – Vishal Dhasal Sep 28 '20 at 08:05
  • To do that I will be needing real object of new SMTPEmailSender and as you can see I will have to call SMTPEmailSender(_mockPdfCreator.Object) which takes mock pdfcreater – Vishal Dhasal Sep 28 '20 at 08:09
  • Yes, exactly. My suggestion would be to get rid of the mock pdfcreator and inject the actual object instead (if that is possible): new SMTPEmailSender(new PDFCreator()) or something like that. – jeroenh Sep 28 '20 at 08:11
  • `(...) get rid of the mock pdfcreator and inject the actual object instead: new SMTPEmailSender(new PDFCreator()) or something like that.` - that is the one of the worst hint you could give Vishal. There's everything ready for testing in isolation. He can test SMTP(..)BuildEmailInfo with IPDFCreater mock to see if BuildEmailInfo works properly, and can test actual implementation of IPDFCreater WITHOUT SMTPEmailSender. It is possible to mix these tests together but what for? It will be harder to tell if fault is in class1 or class2 if any issues arise. Vishal just needs to fix his test setup. – quetzalcoatl Sep 28 '20 at 08:36
  • @jeroenh Your solution did worked but with some changes but I guess it will create a problem for the methods that needs to be mocked anyway thanks .I can you use your answer for other scenarios – Vishal Dhasal Sep 28 '20 at 08:45
  • @quetzalcoatl I didn't say definately get rid of the mock. But if you can configure the actual object to do the same as the mock (and just as fast), why bother? As long as your test remains fast (no external dependencies and no I/O) I argue it's easier to understand without the mock. Opinionated, I know, but I've seen far too many projects with way too many useless and brittle unit tests... – jeroenh Sep 28 '20 at 08:45