Naming for unit tests for controller to edit posts

I am relatively new to TDD and am trying to adopt better names for my tests.

I wrote the following tests abruptly and have since refactored them to the best of my ability, but despite my best efforts, the test names are still (mostly) abysmal.

I am here today to ask that you review my test names.

```
[Test]
public void Edit_ReturnsCorrectView()
{
var actual = controller.Edit("dynamic-contagion-part-one") as ViewResult;

Assert.NotNull(actual);
Assert.That(actual.ViewName, Is.EqualTo("Edit"));
}

[Test]
public void Edit_ReturnsCorrectModel()
{
var viewResult = (ViewResult) controller.Edit("dynamic-contagion-part-one");
var actual = viewResult.Model as PostInputModel;

Assert.NotNull(actual);
Assert.That(actual.Title, Is.EqualTo("Dynamic contagion, part one"));
}

[Test]
public void Edit_NonExistentPost_ReturnsNotFound()
{
var actual = controller.Edit("non-existent-slug") as HttpNotFoundResult;

Assert.NotNull(actual);
Assert.That(actual.StatusDescription,
Is.EqualTo("You cannot edit a post that does not exist."));
}

[Test]
public void Edit_Post_ReturnsCorrectView()
{
var actual = controller.Edit(post) as ViewResult;

Assert.NotNull(actual);
Assert.That(actual.ViewName, Is.EqualTo("Edit"));
}

[Test]
public void Edit_EditsPostContent()
{
post.Content = "arbitrary content.";
controller.Edit(post);

var amendedPost =
repository.Object.Posts.First(p => p.Slug == "dynamic-contagion-part-one");
Assert.That(amendedPost.Content, Is.EqualTo("arbitrary content."));
}

[TestCase("foo", ExpectedResult = "foo")]
[TestCase("foo\r\nbar", ExpectedResult = "foo")]
public string Edit_EditsPostSummary(string content)
{
post.Content = content;
controller.Edit(post);

var amendedPost =
repository.Object.Posts.First(p => p.Slug == "dynamic-contagion-part-one");
return amendedPost.Summary;
}

[Test]
public void Edit_EditsPostSlug()
{
post.Title = "Hello, World";
cont

Let me start out by saying that your test names are actually decent. You have already come to the conclusion that a good test name is one that can convey what is being tested without requiring someone to actually read the test.

As with many things, there are three important pieces of information about what a test is doing:

• What is the thing being tested?

• What is the input to the thing being tested? / In what context is the thing being tested?

• What is the expected result?

If I see a test is failing and the name provides all this information, I can start formulating an idea of what the issue might be without doing more than seeing the test report.

There are a few different theories about how these three things should be ordered based on what the person feels is the focal point. But I prefer the order introduced by Roy Osherove, which matches the order I listed above. This provides us with:

[Test]
public void UnitOfWork_StateUnderTest_ExpectedBehavior()
{
  // ...
}

You don't have to be concerned with the name becoming very wordy. These are test cases and don't need to follow the same naming conventions you would use for methods in your production code. Artificially restricting yourself to HTTP verbs is preventing you from accurately describing the context that is being tested.

Additionally, try to stay away for using "correct value" as the description of the expected result. If I didn't write the code or tests, I likely don't know what the expected result is. More importantly, I won't know why that is the expected value.

Bellow are a few examples of your test case names converted into this style:

• Edit_ReturnsCorrectView -> Edit_SomeSlugName_ViewNameEdit

• Edit_Post_ReturnsCorrectView -> Edit_SomePost_ViewNameEdit

• Edit_EditsPostContent -> Edit_PostWithAlteredContent_PostsContentUpdated

The use of Some is also taken from Roy. It is meant to indicate that there is nothing special about this value and a different arbitrary value should have the same result.

Other notes:

var actual = controller.Edit(model) as HttpNotFoundResult;
Assert.NotNull(actual);

and

var actual = controller.Edit(model);
Assert.NotNull(actual);

Are two very different things. The second case uniquely states that the result must not be null. However, the first case can fail the null check for two different reasons (the result is null or the result is of the wrong type). I suspect you are using this just ensure that the correct type is returned. If that is the case, you should be able to use the testing framework to verify the result is the correct type instead of potentially introducing a null.

Most of your tests are checking that some text field contains an expected sentence. This type of test can be very fragile. If the message displayed to the user is reworded or translated to a different language, the test would start failing. Do you actually care about the wording of the error message or is knowing that a HttpNotFoundResult instance was returned sufficient? Is there an IsError property you could test instead?

Your tests are using a mock repository that is filled with some sample data. There is nothing wrong with this. The one thing I would change would be to move all of the hard coded strings from the tests into the class that produces the mock as constants. This way, all of the data, is defined in one places and you don't have to edit this test class if a different test class requires the repository to contain something slightly different.

StackExchange Code Review Q#57597, answer score: 10