Return to Answer

Mock commands, not queries##

Mock commands, not queries

As Phil Sandler said, the code here is so thin that it's questionable how much value unit tests actually add. However, I would say that their two main benefits are both still relevant:

Putting aside whether you adhere to CQS, or how, the idea of everything being either a command or a query is a useful one in guiding how you test it.

As Phil Sandler said, the code here is so thin that it's possibly questionable how much value unit tests actually add. However, I would say that their two main benefits are both still relevant:

Putting aside whether you adhere to CQS, or to what extent, the idea of everything being either a command or a query is a useful one in guiding how you test it.

• Index_ReturnsCorrectModel is quite confusing. What does "returns correct model" mean? If you were documenting what Index does, you wouldn't have a bullet point reading "returns correct model", in the same way that if you were writing a method that solves some mathematical problem you wouldn't write a test called ReturnsCorrectAnswer.

  What it actually seems to be testing is that it returns models of the correct type. I guess there's nothing wrong with this, but it seems like a pretty low-value test. I can't think of any situation where just returning the correct type is enough to say the method is returning the correct thing, and as soon as you start testing more specific properties, you'll end up casting to the type anyway, so you'll end up testing that implicitly.

• Small point, but it seems more appropriate to pass PostsRepositoryTest's posts in to SetUp as a parameter, rather than have it as a class-level variable. In this case I'm not sure it'll actually make any difference, but it's the kind of thing that sometimes comes back to bite you later, so at the very least it's a good habit to develop.

• GetAllPublished_ExcludesDrafts isn't actually testing what it claims quite as well as it could. If posts have some kind of unique ID property, I'd suggest using that here. Select out the IDs of the returned posts and check that they're identical to the hard-coded IDs of the desired posts. Alternatively I think the way you're setting up the underlying queryable should preserve reference equality, so you could use that to check the same thing. At the very least, you should probably check that .Any(post => post.Draft) is false, though this would mean two asserts in the same test.

• Index_ReturnsCorrectModel is quite confusing. What does "returns correct model" mean? If you were documenting what Index does, you wouldn't have a bullet point reading "returns correct model", in the same way that if you were writing a method that solves some mathematical problem you wouldn't write a test called ReturnsCorrectAnswer.

  What it actually seems to be testing is that it returns models of the correct type. I guess there's nothing wrong with this, but it seems like a pretty low-value test. I can't think of any situation where just returning the correct type is enough to say the method is returning the correct thing, and as soon as you start testing more specific properties, you'll end up casting to the type anyway, so you'll end up testing that implicitly.

  It's just a possibility, but perhaps you arrived at this test through wanting to be TDD-y, and thinking something like "I'm going to need a Post type, so I'd better write a test that will then require that type in order to be made to pass." If so, this is going about TDD the wrong way around! Just because you write the test first, doesn't make it TDD. If you write a test purely to deliberately motivate some design decision you already made, then that's the invented design driving the tests. TDD should be about the tests driving you to discover the design.

• Small point, but it seems more appropriate to pass PostsRepositoryTest's posts in to SetUp as a parameter, rather than have it as a class-level variable. In this case I'm not sure it'll actually make any difference, but it's the kind of thing that sometimes comes back to bite you later, so at the very least it's a good habit to develop.

• GetAllPublished_ExcludesDrafts isn't actually testing what it claims quite as well as it could. If posts have some kind of unique ID property, I'd suggest using that here. Select out the IDs of the returned posts and check that they're identical to the hard-coded IDs of the desired posts. Alternatively I think the way you're setting up the underlying queryable should preserve reference equality, so you could use that to check the same thing. At the very least, you should probably check that .Any(post => post.Draft) is false, though this would mean two asserts in the same test.