That's not more readable to me. Now it's confusing. Your log shows that the "login is ok for testuser" before it checks to see if it's ok. I understand your reasoning for moving it, but in that instance your now causing problems with the log. If I go to look at the log, I'll see that the login was ok, when it may or may not have been.
"Do we really need a double test with two different values? I’m partial to this."
So am I, but in most Test Frameworks this doesn't require you to write a separate test (i.e. xUnit.net has Theory/InlineData for that)
Michael:
putting it up there is not a FINAL solution. it is an increment of the full solution. it is there to force me to create a test that proves it should *not* be there, for all the reasons you mention.
it *should* stand out like a sore thumb, but the only way to cure that pain is with a test.
You actually wanted to reply to J.R. Garcia :)
My 2 cents: Now that you moved the logger to the top, you can write another test that verfies the logger is NOT called if the login is not OK - you've just successfully added another failing test to your arsenal, which you can then refactor with confidence.
Shorter? Yes
public bool IsLoginOk(string user, string password)
{
log.Write("login ok for user: testuser");
return m_users[user] != null && m_users[user] == pasword;
}
would have to switch it back when you want to just log when login is ok though... but still. Shorter == yes :p
excellent advice, thanks a lot!
Roy:
Ok. Then we can completely agree. I think those 4 questions are great
You might want to consider discussing the "refactor" in "red-green-refactor" to avoid confusing readers. After a test passes the next step is to refactor to remove duplication.
> (in tdd you can only change production code functionality when you have a failing test)!
That's not at all true. Anytime all tests are passing it's always OK to refactor to improve the design which would include simplifying the solution.
I don't think your example was very good, had you done the simplest thing that could possibly work, you'd never have to go back and try to hard code something because you'd have hard coded it immediately to pass the first test.
DTSTTCPW is about making the smallest change possible to make a failing test pass, that's all. It doesn't apply to the refactoring stage where you're trying to make the code cleaner or improve the design. It only applies to the make the failing test pass stage.
What about using a randomly generated user name in the unit test? Then you will be forced not to hard code the user name.
Great post, good advice.
Thanks,
Lee
@Ramon
The didactic cause of this post is served well by the example. The point is to help people (Roy's students specifically) characterize "the simplest thing that could possibly work".
@Roy
Do the questions you ask at the end of your post about the efficacy of the new tests point us to refactoring the original test, i.e. Should_Log_After_Successful_Login, to enforce a strict ordering of the expectations? This would then might force a refactoring to e.g. a loggedin event. In truth the original test doesn't really specify the behavior properly, does it?