Reputation: 85875
I am not sure what I should be doing here. Should I be hardcoding all the values in or should I have them in a CONST variables. Everything I seen seems to hard code the values in so I am not sure.
Like this is what I was doing now.
Say in my controller I had a validation test to check if the user tries to submit a form with a blank field.
Now I would have a if statement checking for blank or null variable. If this happened then I would add the error to the ModelState with an error message that I wrote.
so in my unit test I want to make sure that if a blank form variable is submitted that it gets caught.
now in my unit testing I just made a CONST varible and copied in and pasted the validation message in.
So in my assert I compare what the actual message is compared to the message stored in my CONST Varrible. I do this by calling like the Model state and call the field up where I expect the error to be.
Like:
result.ViewData.ModelState["username"].Errors[0];
So if the message is there then it must have gone into my code otherwise it would not exist.
So it occurred to me maybe I should make a new class that will be static and hold all these CONST variables.
That way both the controller views and the unit tests can use them. That way if I got to change the error message then I only need to change it one place. Since I am not testing what the error message is I am testing if it gets set.
The same thing say for exceptions I have some custom messages but I am not testing if the message is right, more if the expection got caught.
The way I am testing it though is to see if the message is the message that I expect since if it is not the message or the message does not exist then something went wrong.
I am new to unit testing so I wanted to make sure that what I am going to do won't some how screw up my unit tests.
To me it makes sense but I thought better check first.
Thanks
Upvotes: 1
Views: 167
Reputation: 233487
It's important to write each test in a way that is robust to subsequent change. You will often need to change parts of your application at a later date, and each time you do that, there's a risk that you will break one of more of your tests.
If your tests are robust to change, a failing test will truly indicate a regression bug.
However, if your tests are what's called Overspecified Tests, every little change you make in your code base may cause tests to fail - not because there was a regression bug, but because the test is too brittle. When this happens, you lose faith in your tests; test maintainance takes a lot of time, and you'll eventually end up abandoning the test suite altogether.
As I read your question, you are already beginning to see the contours of this anti-pattern. I assume that is why you don't test for the specific texts returned, but merely whether they are being set at all. I think this is correct - I rarely test for specific strings, but rather whether a string is specified at all. This makes the test more robust to change, and you avoid the Overspecified Test anti-pattern.
In many cases, instead of doing an Assert.AreEqual on two strings, you can just use Assert.IsNotNull
, or perhaps Assert.IsFalse(string.IsNullOrEmpty(result))
(your platform seems to be .NET).
In general, assertions based on Derived Values are very robust to change, so you may want to take a look at the following blog post:
If you feel particularly adventurous, I can only recommend that you read xUnit Test Patterns, from where many of the patterns and anti-patterns I mention originate. The Art of Unit Testing is also good...
Upvotes: 1
Reputation: 285077
"Should I be hardcoding all the values in or should I have them in a CONST variables."
I'm not sure what you mean. It is fine to have const variables within the unit test class. E.g.:
class FooTest
{
private static readonly string FOO_MESSAGE = "BAR";
or
public static const string BAZ_MESSAGE = "BOO";
...
Assert.AreEqual(FOO_MESSAGE, e.ToString());
}
"So it occurred to me maybe I should make a new class that will be static and hold all these CONST variables.
That way both the controller views and the unit tests can use them. That way if I got to change the error message then I only need to change it one place."
This seems wrong. The mechanism the controller uses to output errors is private to it. That should not be unnecessarily exposed to the unit tests. By doing it that way, you obviously make it more likely that errors will go uncaught. The controller and the tests will "agree" by virtue of the fact they're using the same semi-public mechanism.
"Since I am not testing what the error message is I am testing if it gets set."
If you are really not testing the message, why should the test even know it?
Upvotes: 1