13

I am trying to unit test raising of the event SmtpClient.SendCompleted from a unit test, and I am running into an annoying problem with the test continuing to process before the event can actually fire, terminating the application without actually getting to the event. So, imagine the following code:

[TestClass]
public class emailTest
{
    public bool sentEmail = false;

    [TestMethod]
    public void sendEmail()
    {
        SmtpClient smtp = new SmtpClient("smtpserver");
        smtp.SendCompleted += delegate(Object sender, System.ComponentModel.AsyncCompletedEventArgs e) { sentEmail = true; };
        MailMessage mm = new MailMessage("from@address.com", "to@address.com", "test subject", "test body");
        smtp.SendAsync(mm, "test");
        Assert.IsTrue(sentEmail);
    }
}

This test fails, however, if I manually insert a delay like so...

[TestClass]
public class emailTest
{
    public bool sentEmail = false;

    [TestMethod]
    public void sendEmail()
    {
        SmtpClient smtp = new SmtpClient("smtpserver");
        smtp.SendCompleted += delegate(Object sender, System.ComponentModel.AsyncCompletedEventArgs e) { sentEmail = true; };
        MailMessage mm = new MailMessage("from@address.com", "to@address.com", "test subject", "test body");
        smtp.SendAsync(mm, "test");
        System.Threading.Thread.Sleep(50000); // Manual Delay
        Assert.IsTrue(sentEmail);
    }
}

Then the test passes.

having the method await smtp.SendAsync by wrapping it as a task doesn't seem to actually work because I'm not actually waiting on SendAsync, I'm trying to wait for SendCompleted to finish executing before proceeding with the rest of the test, and I'm not quite sure how to do it.

For time reasons, it's very important that I wait only the minimum amount of time for SendCompleted to finish processing.

I did a whole lot of searching and just can't seem to find anything that addresses this specific issue.

quick edit: in all cases, the email successfully sends, it's only the test that fails.

shf301
  • 31,086
  • 2
  • 52
  • 86
user3657661
  • 306
  • 3
  • 13
  • Is there a reason that you are testing `SmtpClient`? It's not code that you wrote so what is the point in testing it? – shf301 Aug 26 '15 at 19:03
  • I am not testing SmtpClient, I know it works. I am testing the code in the custom handler. – user3657661 Aug 26 '15 at 19:15
  • You should just mock the `SmtpClient` in your test so your test isn't dependent on the behavior of the `SmtpClient`. Like the example in http://stackoverflow.com/questions/19971364/how-to-mock-fake-smtpclient-in-a-unittest. – shf301 Aug 26 '15 at 19:29
  • How does creating a facsimile of the SmtpClient help me with the timing of the events in the actual SmtpClient? I'm not testing whether or not the event fires, or whether or not SmtpClient actually does anything at all. I'm testing the timing of the event firing. If I cannot solidly lock down when the event fires, then I'll have to proceed with inserting a manual delay which is suboptimal. – user3657661 Aug 26 '15 at 20:03
  • Your test shouldn't be on the actual timing of the the SmtpClient. You cannot control this - it is an implementation detail of the framework. It's not useful to test code outside of your control. With a mock SmtpClient you can control the timing (e.g. fire the SendCompleted event immediately) so you can be sure that your SendComplete handler runs immediately. – shf301 Aug 26 '15 at 20:16

1 Answers1

16

Well, I must admit that the fact that SendCompleted is only fired after SendAsync returns sounds a bit odd... it does make unit testing harder.

But if you want to wait the minimum amount of time, you'll have to introduce a synchronization primitive. AutoResetEvent sounds like a good fit for this scenario.

// Arrange
var are = new AutoResetEvent(false);

var smtp = new SmtpClient("smtpserver");
smtp.SendCompleted += (s, e) => { are.Set(); };
var mm = new MailMessage("from@address.com", "to@address.com", "test subject", "test body");

// Act
smtp.SendAsync(mm, "test");

// Assert
var wasSignaled = are.WaitOne(timeout: TimeSpan.FromSeconds(1));
Assert.True(wasSignaled);
dcastro
  • 66,540
  • 21
  • 145
  • 155
  • I just tried this and it didn't work, but I just put that 50 second delay back in right before the Assert and the test passed if I checked my boolean but not if I checked wasSignaled. i.e. smtp.SendCompleted += (s, e) => {are.Set(); sentEmail = true; }; – user3657661 Aug 26 '15 at 19:22
  • 4
    @user3657661 what do you mean by "it didn't work". That does nothing to move the conversation forward. This question has been answered two times: In this answer (which works) and in the duplicate that I applied. – usr Aug 26 '15 at 19:28
  • By "it didn't work" I obviously mean the test failed. This answer clearly does not work, and I can prove it. observe this link: http://imgur.com/a/XtzVg If you're going to claim code objectively works when people claim it doesn't, you could at least bother to test it yourself, it's only a few lines. – user3657661 Aug 26 '15 at 20:00
  • 3
    Probably the timeout set to 1sec in this piece of code is too short. If you take the time to *understand* what it's doing you can change the timeout to infinity.; You haven't responded to the duplicate either. – usr Aug 26 '15 at 20:08