Richard Lowe wrote:
> Ali Bahrami <Ali.Bahrami at Sun.COM> writes:
>> I pulled over scmtest and ran it, and it did in fact pick up
>> something that I had changed. The cddlchk test relies on
>> Cddl.py having a variable Cddl.CmntChrs, and in my changes, I
>> had refactored that into CmtBlk.py (and so, CmtBlk.CmntChrs).
>>
>> So I added Cddl.CmntChrs back (taking its value from CmtBlk.py),
>> and now all the tests pass without modification. I'm very glad I
>> ran these before integration...
>
> I'm confused by that. If you mean the *test* relies on that, surely
> fixing the test is the right thing to do?
>
> If Cddl relies on that, I'd assume it was using your CmtBlk code now.
>
>> I looked into adding a mapfilechk test, and it is truly very
>> easy. However, the result is a cut down copy of the cddlchk
>> test that does not actually test anything new. The reason for
>> this is that Cddl.py and Mapfile.py are now one liners that
>> reference CmdBlk.py (which is where all the guts of Cddl.py
>> hve moved). Hence, testing cddlchk really does fully test
>> mapfilechk as well.
>
> So in that case, rename the cddl tests to indicate they test the cmntblk
> stuff, and comment that what it covers is the implementation of both cddl and
> mapfile?
>
> -- Rich
I mean that the test relies on it. Cddl uses CmtBlk as you assumed.
Cddl is short enough now, so let me just quote it here:
-----
import sys, CmtBlk
# scmtest has a test for cddlchk that depends on the variable
# Cddl.CmntChrs. However, that variable has been refactored into
# CmtBlk. The following line preserves the original interface
# from the Cddl module, and allows existing programs that assume
# Cddl.CmntChrs exists to continue working.
#
CmntChrs = CmtBlk.CmntChrs
# The CDDL string above contains the block guards so that the text will
# be tested by cddlchk. However, we don't want to include the initial
# \n or the block guards in the text passed in.
#
CDDL = CDDL.splitlines()[3:-2]
def cddlchk(fh, filename=None, lenient=False, verbose=False, output=sys.stderr):
return CmtBlk.cmtblkchk(fh, 'CDDL', CDDL, filename=filename,
lenient=lenient, verbose=verbose, output=output)
-----
So you see, Cddl is simply preserving its original interface, using the
value provided by CmtBlk.
Now, fixing the test is the other way to go. However, CmtBlk is really an
implementation detail of Cddl. I don't quite see that a user of Cddl
should have to know about CmtBlk. And, if I do fix the test, then I have
to coordinate the putback for the tests with the tools, and there's always
the chance that I'll break someone who has a slightly old copy of the tests.
This way, I'm confident that if Cddl worked for you before, it still does.
After chewing on that for a bit, I just felt that this was the better
way to go. Let me know if you feel that I'm off base...
- Ali
