>> http://cr.opensolaris.org/~mjnelson/webrev.456.457/
>>
>> I updated the rtichk.py test, too; a couple of the existing tests seemed
>> (to me) to be incorrect.  For the off-SWAN check, I instrumented
>> DbLookups.py to always throw RtiOffSwan, and called rti with multiple
>> bugids.
>>
>> For the consolidation specified but gate path not specified, I added
>> another test to rtichk.py.  If those conditions provide insufficient
>> scoping, such that multiple lines are returned, then this will still fail
>> with invalid output.
>
> Could you move the RTI tests into tooltest?  I think I left a stub
> there, that removes itself if not onSWAN(), filling that out (leaving
> the onSWAN bit), should be easy, I think?  I held off because I
> wouldn't be able to run them, but they all need to be in the same
> place.

I did this, but it will require at least a little bit of review to make 
sure it's doing what's expected.  I believe I got the existing checks from 
usr/src/tools/onbld/Tests/rtichk.py migrated correctly (and even fixed, in 
a couple cases), but I'm not convinced they're a particularly robust set 
of tests to start with.

> Rti.py:94
>  Does that sit nicely with cdm, etc?  And not over-report that as an
>  error?

We were previously reporting the off-SWAN error once for each cr, now 
we're only reporting it once for the whole thing.  In conjunction with 
that reporting, we were previously appending to badRtis, which would cause 
us to return False later; now we're returning False right away.  So from 
the cdm standpoint, nothing has changed.

>  (I have vague memory that one of the problems johnlev fixed here was
>  regarding us doing too much in that area, but I'm no longer certain)
>
> DbLookups.py:339
>
>  We don't necessarily use webrticli, in fact, in the ideal common
>  case it won't run at all.
>
>  This is the bit we talked about briefly on IRC.  I'm no longer sure
>  *why* it attempts a direct connection and only falls back upon
>  webrticli.  But if we keep doing that, it'd probably be best to
>  state such.

Right.  Comment fixed.

> DbLookups.py:346 (not part of your change, exactly)
>
>  Why in the world do we default these keyword args to "NULL" rather
>  than None?
>
>  It's claimed that this is how webrti wants us to do it, but that's
>  not what the code actually does, the code in both paths then goes !=
>  "NULL", which seems to suggest we're using "NULL" as a surrogate for
>  None for seemingly no reason.

I fixed this up.

Updated webrev:

        http://cr.opensolaris.org/~mjnelson/webrev.456.457.v2/

Reply via email to