Keith,
Thanks. See below.
John
On 01/ 3/11 01:21 PM, Keith Mitchell wrote:
A test covering the failure case (where aisvc.lookup() returns -1)
should probably be added.
Added:
assert aisvc.lookup() != -1, \
'lookup did not succeed for service (%s)' %
self.SERVICE_NAME
Sorry for not being clear; I was looking for a separate test case /
function that would create a scenario in which the assertion:
assert aisvc.lookup() == -1
*succeeds*. When I look at the code again, I see that it was actually
already covered (lines 83-84 of the updated webrev); so if that logic
could be pulled out to a separate case so that the different scenarios
are tested separately, that'd be great.
Added another function to pull out the 'assert aisvc.lookup() == -1' as
well as
the 'assert aisvc.found == -1'.
273: send_needed_criteria() could use tests for the error cases:
- Non-existent path based on the port
- Error occurs during lines 114-115 of cgi_get_manifest.py
Yes. I ran into some AI db issues. Still haven't worked around
those yet.
347: send_manifest() could use tests for its error cases.
Yes. Same AI db issues.
Ok. Sue may have some ideas in that area, in terms of getting a mock
DB going. This may be an area which would require refactoring of
existing code to be easily testable; if you can't get something
working quickly, I think we'll be ok (but file a CR so it can get
looked at later).
I will file a bug report for this issue.
list_manifest() may benefit from some additional cases, though that
one seems like a less-used interface?
Yes. It is only used by system administrators. It is like
installadm list.
cgi_get_manifest.py:
While looking at this file in relation to the test classes, I
noticed that lines 216 and 221 might benefit from using
"some_string".partition() (rather than .split()). I'm also not
certain, but it seems like resetting "criteria = {}" on line 224
could be a bug?
This particular part of the code is actually from the original
cherrypy webserver.py
code. It is unchanged. The lines following (226-229 and 242) handle
the case where
criteria is an empty dictionary:
# find the appropriate manifest
try:
manifest = AIdb.findManifest(criteria, aisql)
except Exception, err:
....
if str(manifest).isdigit() and manifest > 0:
If it's existing code that's been working, than no worries.
Yep, code that has been working.
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
