Are there too many asserts in this unit test?_问答_开发者

Are there too many asserts in this one unit test?

[Fact]
public void Send_sends_an_email_message() {
    using (var server = new MockSmtpServer()) {
        server.Start();
        using (var client = new EmailClient("localhost")) {
            string from = "john.doe@example.com";
            IEnumerable<string> to = new[] { "jane.doe@example.com" };开发者_StackOverflow中文版
            string subject = "Test";
            string body = "Test.";
            client.Send(from, to, subject, body);
            var session = server.Sessions.FirstOrDefault();
            Assert.NotNull(session);
            var message = session.Messages.FirstOrDefault();
            Assert.NotNull(message);
            Assert.NotNull(message.From);
            Assert.Equal(message.From.Address, "john.doe@example.com");
            Assert.NotNull(message.To);
            var recipient = message.To.FirstOrDefault();
            Assert.NotNull(recipient);
            Assert.Equal(recipient.Address, "jane.doe@example.com");
            Assert.Equal(message.Subject, "Test");
            Assert.Equal(message.Body, "Test.");
        }
    }
}

I don't think this code requires any explanation, but if it does please let me know.

I try to keep my UnitTests fairly small and test one thing at a time. So I'd likely make testing distinct parts into separate tests, e.g.

sendWillSendAnEmail,
fromContainsSenderAddress,
toContainsRecipientAddress,
mailBodyContainsMailMessage,
mailContainsSubject

I think it's too large.

When you have a bunch of smaller tests, you can get "defect localization" - just by running all the tests, you can see exactly where a problem is. With as many asserts as you currently have (and no assert messages) you'd probably have to launch a debugger to find out. Remember that you probably will end up having hundreds if not thousands of tests, and if a bunch of them are failing, you don't want to have to debug each one to see why.

Also, any assert failing early in the test means the later asserts are not run. When they're separated into individual tests, every assert is checked. This is sort of a trade-off; a lot of those asserts probably are related and will fail at the same time, so you'll have five red tests instead of one. But I work from the premise that more information is better, so I would rather have those five tests and know that all five asserts failed.

I see no particular problem with your asserts, but if you want to clean up your code, you could change

var session = server.Sessions.FirstOrDefault();
Assert.NotNull(session);

into

var session = server.Sessions.First();

First() will throw an exception anyway, so by just changing your code you get the benefit that the assert would bring you without as much code. There are other places you can make similar changes too.

But as a general rule, take nothing for granted in unit tests -- and that means lots of asserts!

It depends on what standards you're adhering to, but I'd generally say yes, you've got far too many asserts in that test.

Many folks contend that a single test should have a single assert; I think that can be a bit of overkill, but I do certainly believe that it's appropriate to have a single unit test for a single "chunk" of functionality; you're all over the place with your asserts. The test is too big; break it up into several different tests.

No in general the more assertions the better. A common mistake in unit tests is not being explicit enough.

Your test is very explicit and readable. I especially like the assertions on null. It is a good practice because it makes interpreting a test failure extremely simple.

The only way you can have too many assertions is if you assert the exact same thing more than once, which you are not doing.

Yes, you have too many asserts in your code! Moreover, assert statement should be only one per test method. Using many asserts may be the code smell that you are testing more than one thing. Moreover, there is a chance that somebody can add new assert into your test instead of writing another one. And how can you understand how your other asserts completed when the first one failed?

You may also found interesting this post: https://timetocode.wordpress.com/2016/06/01/zen-of-unit-testing/

I think the question is, do the assert()d values change independently? If they change independently, then they should be tested in different tests that vary the conditions related to each assert.

If however you have a single codepath that is generating a an email with all those fields, then testing all those things in one test is appropriate.

Now the test is kind of hard to read though. You may want to wrap those asserts in a descriptive helper method. I wouldn't bother with that until I find the same helper method could be useful somewhere else.

Your many assert statements is an indicator of how much logic you have in unit tests. You are going to have to maintain your unit tests like regular code. It is better to spend time on defensive programming and programming by contract than to code unit tests.

Maybe you can create a new Email class that accepts all those parameters in its default constructor.

And then you can throw an exception in case the user passes an invalid parameter.

And in your unit test

Send_sends_an_email_message

you can add the asserts that checks equalities only instead of checking against NULLs.

Maybe you can create 2 emails in one test and do the equality asserts for both instances.