[DTrace-devel] [PATCH] test: add test for probe* bvar support for pid probes

[Kris Van Hees]

On Thu, Sep 08, 2022 at 05:12:48PM -0400, Eugene Loh wrote:
> On 9/8/22 16:33, Kris Van Hees wrote:
> 
> > On Thu, Sep 08, 2022 at 12:37:32PM -0400, Eugene Loh via DTrace-devel wrote:
> > > I'm a little confused.  The subject line says probe*, but the patch only has
> > > probefunc.  The xfail line says probe*.  The body of the commit message says
> > > probefunc.  I'm unclear what the intended scope of the patch is.
> > The test is a representative example case for the problem.  The test exercises
> > the case that is known to fail right now, and it is sufficient because all the
> > probe* bvars are taken from the same source and the problem here is either
> > present for all probe* vars or none.
> 
> Okay, but that leaves open what to do with the patch.  I'd say either:
> 
> *)  (recommended) add the other three "missing" tests
> 
> *)  (dubious) talk only about probefunc, not about probe*

I disagree.  The point here is not to add tests for every provider to ensure
all probe* bvars can be accessed and have sane values.  That might be something
we do want to do.  But this is a patch to introduce a specific test to exercise
functionality that is known to be currently broken.  It primarily serves as a
reminder that we need to fix this.

I don't think that 'you could add more tests' is a valid argument to not give
a r-b.  This is a trivial patch that adds a valid and useful test to ensure we
capture that this functionality does not work yet.

> > > Also, the commit message says "because it does not work right."
> > > Grammatically, "it" would refer to the test, but the message might be
> > > clearer if one indicts specifically the pid bvar support rather than the
> > > test.
> > > 
> > > Possibly, the script should also have a time-out or ERROR probe but that may
> > > be overkill -- I don't have a solid case to make for them.  The test is fine
> > > (in this respect) as is.
> > > 
> > > On 9/7/22 16:15, Kris Van Hees via DTrace-devel wrote:
> > > > This test exercises the correct support for probefunc for pid probes.
> > > > The test is marked @@xfail because it does not currently work right.
> > > > 
> > > > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > > > ---
> > > >    test/unittest/pid/tst.probefunc.d | 26 ++++++++++++++++++++++++++
> > > >    1 file changed, 26 insertions(+)
> > > >    create mode 100644 test/unittest/pid/tst.probefunc.d
> > > > 
> > > > diff --git a/test/unittest/pid/tst.probefunc.d b/test/unittest/pid/tst.probefunc.d
> > > > new file mode 100644
> > > > index 00000000..a8f273cf
> > > > --- /dev/null
> > > > +++ b/test/unittest/pid/tst.probefunc.d
> > > > @@ -0,0 +1,26 @@
> > > > +/*
> > > > + * Oracle Linux DTrace.
> > > > + * Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
> > > > + * Licensed under the Universal Permissive License v 1.0 as shown at
> > > > + * http://oss.oracle.com/licenses/upl.
> > > > + */
> > > > +
> > > > +/* @@runtest-opts: -c /bin/date */
> > > > +/* @@trigger: none */
> > > > +/* @@xfail: dtv2 -- probe* builtin variables do not work for pid probes yet */
> > > > +
> > > > +pid$target:libc.so:malloc:entry
> > > > +{
> > > > +	trace(probefunc);
> > > > +}
> > > > +
> > > > +pid$target:libc.so:malloc:entry
> > > > +/probefunc == "malloc"/
> > > > +{
> > > > +	exit(0);
> > > > +}
> > > > +
> > > > +pid$target:libc.so:malloc:entry
> > > > +{
> > > > +	exit(1);
> > > > +}
> > > _______________________________________________
> > > DTrace-devel mailing list
> > > DTrace-devel at oss.oracle.com
> > > https://oss.oracle.com/mailman/listinfo/dtrace-devel