[DTrace-devel] [PATCH 07/12] Add tests for various options and tunables

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

Plus a bunch of tiny comments:

In the commit message:
     Tests added for:
         -xargref
         -xbufsize               <<< here <<<<<<<<+
         -xcore                                   ^
         -xcppargs                                ^
         -xcpphdrs                                ^
         -xcpppath                                ^
         -xctypes                                 ^
         -xbufsize (and -b)      >>> move this >>>+
         -xdebug
         [...]

In the commit message:
     -   -xbpglog
     +   -xbpflog
     -   -xbpglogsize
     +   -xbpflogsize

The tests for xbpflog[size] have been moved.  I'm curious about the 
scope of test/unittest/options and our test coverage.  There are 
"pragma" tests for bpflog[size].  Should the new directory also have 
"pragma" tests for other options as well?

The test/unittest/options/err.D_EMPTY.errtags.d test is okay, but a 
little weird in that all the err.D_* tests already depend on -xerrtags 
anyhow.  But I guess that's okay... some redundant/superfluous testing.

Instead of fixing files
     test/unittest/options/err.D_PRAGCTL_INVAL.no-cpp.d
     test/unittest/options/tst.cpp.d
just introduce them "correctly" (in the earlier patch in this same series).

The tests
     test/unittest/options/err.b[ufsize]-too-low.d
     test/unittest/options/tst.b[ufsize].d
are nice.  They do, however, depend sensitively on the current 
implementation.  In a way that's okay.  If the implementation changes 
and one forgets to update the tests, at least some test failures will 
remind the developer to update the tests.  But the developer might not 
be sufficiently circumspect to realize some non-failing tests also need 
updating.  So, I suggest adding a comment to all four tests reminding 
developers that all four have to be maintained in sync.

The test test/unittest/options/err.no-errtags.d has the comment:
     ASSERTION: Without the -xerrtags option, errors are printed with tag.
Is that right, or was "s/with/without/" intended?

The tests
     test/unittest/options/tst.D.d
     test/unittest/options/tst.define.d
should be fine, but might as well add .r results files for them, to get 
that special reassurance that :BEGIN shows up.

The tests
     test/unittest/options/tst.ctfpath.sh:
     test/unittest/options/tst.ctypes.sh:
     test/unittest/options/tst.dtypes.sh:
all have dangling tabs (line that's empty other than a tab).

The tests
     test/unittest/options/tst.core.sh
     test/unittest/options/tst.ctypes.sh
     test/unittest/options/tst.dtypes.sh
write files to the current directory.  That seems like a dubious 
practice.  Most *.sh tests that write files write either to $tmpdir or 
to their own $DIRNAME directories.  In particular, there is no guarantee 
that I know of that the DTrace source directory won't ever have, say, 
ctypes.* or dtypes.* files in it (that would then get wiped out by our 
testing).

The test test/unittest/options/tst.core.sh should exit $rc not $rcc.  
Also, out of curiosity, for the awk script, how about:
     /ELF/ && /core file/ && /dtrace/ { exit(0); }
     END { exit(1); }

The test/unittest/options/tst.debug.sh test seems fine, but would you be 
open to the following enhancement?
         - exit(sum > 0 ? 0 : 1);
         + exit(cnt["libdtrace"] > 0 && sum > 0 ? 0 : 1);

The test test/unittest/options/tst.defaultargs2.d should have a .r 
results file.

For the tests
     test/unittest/options/tst.destructive.d
     test/unittest/options/tst.w.d
     test/unittest/options/tst.zdefs.d
     test/unittest/options/tst.Z.d
it would also be nice to have tests that confirm that the absence of the 
option triggers an error -- e.g., the same way there are tests to check 
behavior in the absence of argref, cpp, defaultargs, empty, or errtags.  
That is, it would be nice to have tests like
     err*no-destructive.d
     err*no-w.d
     err*no-zdefs.d
     err*no-Z.d

The
     test/unittest/options/tst.Z.d
     test/unittest/options/tst.zdefs.d
tests are fine, but how about a more-clearly-illegitimate probe name?  I 
could imagine an "oracle" probe.  How about "foo" or 
"nonexisting_probe_name" instead?