On 08/11/10 04:12 AM, Dermot McCluskey wrote:
Keith,
Responding to
test_manifest_serv.py
On 08/09/10 20:11, Keith Mitchell wrote:
[...]
test_manifest_serv.py:
69: Should there be assertions here? If this is relying on an
exception being raised, it'd be slightly preferred to enclose in a
try/except <Specific exception> clause, with a self.fail(<msg>) in
the except block. This differentiates between "expected" failures,
and completely unrelated errors.
No, this does not reply on an exception being raised.
There are two related tests here, the previous one,
test_validate_fails_if_dtd_schema_is_false, replies on an exception
and uses assertRaises(). This test,
test_validate_succeeds_if_dtd_schema_is_true, just depends on an
exception *not* being raised.
Right, sorry, that's what I meant. If the idea is to fail the test if a
specific exception is raised (and succeed if no exception is raised), I
think it's slightly better form to catch that exception and explicitly
fail the test. It's mostly a style issue - it relates back to the fact
that PyUnit differentiates between tests that failed (assertion failure)
and tests that error'ed out 'unexpectedly' (uncaught exception). The
latter could signify a bug in the test code, an issue with the execution
environment, etc.
Again, just a minor point (albeit one that took a bit of explanation!)
There is no return value or instance
variable to assert against so it just passes if not exception was raised.
I will add a try/except clause to make it more clear what the test
is doing.
Thanks,
Keith
Thanks,
- Dermot
installadm.1m.txt:
1-22: Based on other man pages (both in our gate and elsewhere),
should this even have a CDDL block?
On 08/ 5/10 03:07 PM, Ethan Quach wrote:
Hi all,
The following is the code review for the AI manifest schema changes,
and the installadm criteria changes. It is a rather large review, so
partial/piecewise review would be also be fine, just let us know what
you're reviewing. We've pre-requested reviews from some of you
already, but all comments welcomed by Aug 16th.
Webrev:
-------------
http://cr.opensolaris.org/~equach/webrev.ai-schema/
Bugs:
--------
16423 <http://defect.opensolaris.org/bz/show_bug.cgi?id=16423>
Updates to AI schema should be made
15449 <http://defect.opensolaris.org/bz/show_bug.cgi?id=15449>
installadm add validates combined manifest against image-specific
schema as well as schema in /usr/share/auto_install/
6975043 <http://bugs.opensolaris.org/view_bug.do?bug_id=6975043>
separate criteria and ai manifest
thanks,
-ethan
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
------------------------------------------------------------------------
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
