On 08/11/10 16:51, Keith Mitchell wrote:
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.
OK - I got it. I'll call self.fail("...") from the except clause.
Thanks for explaining,
- dermot
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
