[DTrace-devel] [PATCH 09/10] usdt: test improvements

[Kris Van Hees]

On Tue, Aug 29, 2023 at 05:18:47PM +0100, Nick Alcock wrote:
> On 22 Aug 2023, Kris Van Hees outgrape:
> 
> > On Wed, Aug 02, 2023 at 02:26:59PM +0100, Nick Alcock via DTrace-devel wrote:
> >> This soups up test/unittest/usdt/tst.manyprocs.sh to test dtprobed's
> >> cleaning up the wreckage of dead processes, by having every other process
> >> kill itself rather than dying peacefully (and sending a DTRACEHIOC_REMOVE).
> >> When doing in-tree testing (something we can detect via $dtrace), we can
> >> also check the DOF stash itself to see whether the wreckage has been cleaned
> >> up. (This isn't practical when doing systemwide testing because we don't
> >> know how much DOF the dtprobed might legitimately be hanging on to.)
> >
> > I really do not like this...  This should be two tests: one that handles the
> > testing that is meant to pass for installed DTrace and one for non-installed
> > (or in-tree) DTrace.  Having one test means that the behaviour differs based
> > on the way the testsuite is executed and that is confusing.  And besides, the
> 
> I'd call it unavoidable if we want to do whitebox testing of dtprobed at
> all (which we probably do, given that it found a bunch of bugs). You
> can't whitebox test it when it might be doing other work on behalf of
> other things on the system, and we *do* want to use the systemwide
> dtprobed when not doing in-tree testing (when it is going to match the
> tested dtrace) because that's the only one that is managed by systemd,
> and we have had actual bugs caused by that.
> 
> > description clearly shows that different things are tested (or expected to
> > pass) depending on the run mode.
> 
> I thought it might be even more confusing (and prone to skew) to have
> two nearly identical tests with one slightly different bit towards the
> end, one run when in-tree and one when out-of-tree, rather than one test
> with a conditonal near the end that tests in-tree-only stuff. Would you
> *really* find that clearer? (Because that's what I started doing and it
> was... clearly not a good idea. At least not the way I was doing it.)
> 
> >> tst.multitrace.sh gains the ability to test DOF reparsing on upgrade,
> >> waiting until the first probe-containing process has started and then
> >> intentionally overwriting some of the parsed DOF with junk (in lieu of
> >> actually changing struct dof_parsed on the fly :) ) and forcing a reparse
> >> via a kill -USR2 (in lieu of communicating the complete argument list of
> >> dtprobed down to this test just so it can be killed and restarted).
> >
> > Same here.
> 
> So you're suggesting, what? Do a multitrace.sh without a bunch of USR2s
> to force reparses, and then do the *exact same thing* with the USR2s
> involved? The former test would seem entirely redundant in that case
> (and it's not very fast), but the reparsing would be doing no work if it
> wasn't embedded in something that did more or less what
> tst.multitrace.sh does.

Two tests: one runs when not testing in-tree, the other when testing in-tree.
In other words, the in-tree test should be SKIPPED when doing out-of-tree
testing, and the out-of-tree test should be SKIPPED when doing in-tree testing.

Same for the tst.manyprocs.sh test.  That way there is no redundancy and we
still do not have a test that exercises extra things in one case and not in the
other.