hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Sutton <adr...@intencha.com>
Subject Test Coverage
Date Fri, 20 Jun 2003 11:56:40 GMT
Howdy all,
We now have a license for clover to analyze our test cases and am now 
just starting to work through adding test cases to improve our code 
coverage.  I've very quickly come to the realization that 100% code 
coverage may actually be a bad thing.  I've gotten AuthChallengeParser 
to 100% coverage now so let me use it as an example:

There are four test cases that I consider pedantic and 1 of those that 
I really don't like.  The pedantic ones are:

* test that AuthChallengeParser.extractScheme(null) throws an illegal 
argument exception instead of a null pointer exception.  What 
difference does this really make?  The only reason I can think of to 
keep the test is so that the implied interface for the method is kept 
the same (it never throws a NPE but throws an illegal argument 
exception instead).

* Test that AuthChallengeParser.extractParams(null) throws an illegal 
argument exception not a NPE.  Same as above really.

* testParamsWithNoName().  Check that 
AuthChallengeParser.extractParams("Basic realm=\"test\",=\"invalid\""); 
throws a MalformedChallengeException because there is no name part to 
the name/value pair.  This is good if we're in strict mode but if not 
wouldn't it be better to be lenient and just ignore that param (with a 
logged warning)?

The one I really don't like is:

* Test that AuthChallengeParser.extractScheme(" Basic 
realm=\"realm\""); throws a MalformedChallengeException because there's 
a leading space.  Now in strict mode that's fine and within HttpClient 
the leading space is stripped before being passed in but it seems 
overly strict to me.

Now, I don't mind what happens with any of these decisions to be honest 
as none affect the actual behaviour of HttpClient - they are very much 
edge cases.  I would however like to set up a policy on the types of 
test cases I should create (do we want to avoid testing things like the 
pedantic things above) as well as the best way to keep track of 
questionable or overly pedantic test cases.  Currently I'm just adding 
a // pedantic  above any test case that seems pedantic and a todo 
comment over anything that I think may require a change to the code but 
isn't clearly a bug.

I figure from time to time I can provide a list of issues that need to 
be considered as I work my way through the codebase.

Personally, I'm hoping to achieve 100% test coverage firstly because 
I've discovered how dependent I am on having good test cases while 
working on HttpClient (most people don't have the detailed level of 
knowledge that Mike and Oleg do and thus aren't aware that a change 
will break some other section of code - NTLM is a regular victim of 
this).  Also, aiming for 100% coverage makes a very clear-cut decision 
on when the job is done which make life easier as well and makes it 
much more noticeable when new test cases need to be added.

Any thoughts?

Adrian Sutton.

View raw message