Ayende @ Rahien

It's a girl

Defensive coding is your friend

We just had a failing test:

image

As you can see we assumed that fiddler is running, when it isn’t. Here is the bug:

image

Now, this is great when I am testing things out, and want to check what is going on the wire using Fiddler, but I always have to remember to revert this change, otherwise we will have a failing test and a failing build.

That isn’t very friction free, so I added the following:

image

Now the code is smart enough to not fail the test if we didn’t do things right.

Comments

Frank Quednau
02/12/2013 11:23 AM by
Frank Quednau

As with every good advice, there is the evil counterpart: paranoid coding, when you litter your code with excessive null checks on things that cannot be null, catch exceptions you cannot handle, etc.

Jonas
02/12/2013 11:42 AM by
Jonas

I wouldn't call this defensive coding. You created testable code WAY different.

Defensive coding IMHO is ONLY EVIL... But we migh mean different things,. To me defensive coding is what Frank is describing as paranoid coding :)

/Jonas

Gilligan
02/12/2013 12:04 PM by
Gilligan

I did a similar thing recently where I was testing seeding the database. This meant destroying and rebuilding it from NHibernate. However the config file was pointing to the QA database so it got wiped and needed to be restored from backup. Now I have an assertion in TestInitialize that ensures the database is local before seeding it with test data.

Serhiy Kalinets
02/12/2013 02:01 PM by
Serhiy Kalinets

bool args do not add clarity to the code. When you catch GetServerUrl(true) in the code it may be tricky to figure out what that true means without looking at method signature.

Daniel Lang
02/12/2013 03:09 PM by
Daniel Lang

@Serhiy, this is why you can and should use named parameters in these cases, like GetServerUrl(fiddler:true)

Duke
02/12/2013 03:44 PM by
Duke

Mmmm

Now the code is smart enough to not fail the test if we didn’t do things right.

Is that really a good thing? It doesn't sound it.

Feels like the sort of thing which could lead to false positives, thinking you were actually testing something when internally it was circumventing it.

Daniel Wonisch
02/12/2013 07:46 PM by
Daniel Wonisch

Still this seems to me as a nasty workaround instead of a real solution, what might be OK for test code but wouldn't be acceptable in production code.

@Daniel: I have to be on Serhiy side of view. Since you do not want the developer to enable or disable fiddler all the time, but let the system take the decision for you, a better signature for that method would be GetFiddlerUrlOrDefault().

Janus
02/13/2013 08:28 AM by
Janus

Ayende, is that you?....

pd
02/15/2013 12:18 PM by
pd

Just wondering...Ayende writing fragile tests? This is not best practice.

Comments have been closed on this topic.