[DTrace-devel] [PATCH] test: split tst.str_comparison-basic.d into with and without NULL testing

Tue Oct 11 23:03:11 UTC 2022

On Tue, Oct 11, 2022 at 06:43:13PM -0400, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

Thanks.

> but:
> 
> 1) s/fialing/failing/

Thanks.

> 2) One or both tests need copyright year update?

Ah yes.

> 3)  The naming strikes me as quite odd.  I would think the "basic" name
> would be retained for the original test;  after all, that is the version we
> might expect to last into "perpetuity."  The "workaround" version without
> NULL pointers should be the new test, with some reasonable hope that we will
> retire it in the not-too-distant future.  Maybe even
> tst.str_comparison-basic-tmp.d.

Well, as it turns out (in terms of how strings are handled in DTrace), NULL is
a very special case for strings.  So, I'd say that the basic comparison testing
not including NULL is reasonable because (1) it cannot work because we do not
implement NULL strings yet, and (2) NULL strings are special enough to call
them out specifically in extra testing.  I am not doing this as a workaround
but simply based on the fact that tst.str_comparison-basic.d (before this
patch) cannot PASS and thus it is not giving us any real indication of whether
strcmp() works or not.  I.e. it appears as if it does *not* work while it
actually does.  That's a problem.

Therefore, I propose to change tst.str_comparison-basic.d into the real basic
testing, and have tst.str_comparison-basic-with-NULL.d as an expanded testing
(@@xfail for now) because it tests more than is currently implemented.

I would not intend to remove either in the future, especially because the NULL
string support will remain an implementation thing on its own and therefore
likely to cause problems (if there is abug) unrelated to strcmp() itself.  That
is good to differentiate on.

> On 10/11/22 17:40, Kris Van Hees via DTrace-devel wrote:
> > Since NULL strings are not supported yet, the tst.str_comparison-basic.d
> > test is fialing even though the non-NULL case should PASS without any
> > problem.  The test is now written as two tests: one that exercises the
> > case of NULL strings and one that does not.
> > 
> > Signed-off-by: Kris Van Hees<kris.van.hees at oracle.com>
> > ---
> >   .../tst.str_comparison-basic-with-NULL.d      | 75 +++++++++++++++++++
> >   .../operators/tst.str_comparison-basic.d      | 23 ------
> >   2 files changed, 75 insertions(+), 23 deletions(-)
> >   create mode 100644 test/unittest/operators/tst.str_comparison-basic-with-NULL.d
> > 
> > diff --git a/test/unittest/operators/tst.str_comparison-basic-with-NULL.d b/test/unittest/operators/tst.str_comparison-basic-with-NULL.d
> > new file mode 100644
> > index 00000000..64bb0579
> > --- /dev/null
> > +++ b/test/unittest/operators/tst.str_comparison-basic-with-NULL.d
> > @@ -0,0 +1,75 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2021, 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.
> > + */
> > +/* @@xfail: No support for NULL strings yet */
> > +
> > +/*
> > + * ASSERTION: String comparisons work.
> > + *
> > + * SECTION:  Operators
> > + */
> > +
> > +#pragma D option quiet
> > +
> > +BEGIN
> > +{
> > +	nerrors = 0;
> > +
> > +	s1 = "abcdefghi";
> > +	s2 = "jklmnopqr";
> > +	s3 = "stuvwxyz!";
> > +
> > +	nerrors += (s1 <= s2 ? 0 : 1);
> > +	nerrors += (s1 <  s2 ? 0 : 1);
> > +	nerrors += (s1 == s2 ? 1 : 0);
> > +	nerrors += (s1 != s2 ? 0 : 1);
> > +	nerrors += (s1 >= s2 ? 1 : 0);
> > +	nerrors += (s1 >  s2 ? 1 : 0);
> > +
> > +	nerrors += (s2 <= s2 ? 0 : 1);
> > +	nerrors += (s2 <  s2 ? 1 : 0);
> > +	nerrors += (s2 == s2 ? 0 : 1);
> > +	nerrors += (s2 != s2 ? 1 : 0);
> > +	nerrors += (s2 >= s2 ? 0 : 1);
> > +	nerrors += (s2 >  s2 ? 1 : 0);
> > +
> > +	nerrors += (s3 <= s2 ? 1 : 0);
> > +	nerrors += (s3 <  s2 ? 1 : 0);
> > +	nerrors += (s3 == s2 ? 1 : 0);
> > +	nerrors += (s3 != s2 ? 0 : 1);
> > +	nerrors += (s3 >= s2 ? 0 : 1);
> > +	nerrors += (s3 >  s2 ? 0 : 1);
> > +
> > +	s2 = NULL;
> > +	nerrors += (s3 <= s2 ? 1 : 0);
> > +	nerrors += (s3 <  s2 ? 1 : 0);
> > +	nerrors += (s3 == s2 ? 1 : 0);
> > +	nerrors += (s3 != s2 ? 0 : 1);
> > +	nerrors += (s3 >= s2 ? 0 : 0);
> > +	nerrors += (s3 >  s2 ? 0 : 0);
> > +
> > +	nerrors += (s2 <= s3 ? 0 : 1);
> > +	nerrors += (s2 <  s3 ? 0 : 1);
> > +	nerrors += (s2 == s3 ? 1 : 0);
> > +	nerrors += (s2 != s3 ? 0 : 1);
> > +	nerrors += (s2 >= s3 ? 1 : 0);
> > +	nerrors += (s2 >  s3 ? 1 : 0);
> > +
> > +	s3 = NULL;
> > +	nerrors += (s2 <= s3 ? 0 : 1);
> > +	nerrors += (s2 <  s3 ? 1 : 0);
> > +	nerrors += (s2 == s3 ? 0 : 1);
> > +	nerrors += (s2 != s3 ? 1 : 0);
> > +	nerrors += (s2 >= s3 ? 0 : 1);
> > +	nerrors += (s2 >  s3 ? 1 : 0);
> > +
> > +	printf("%d errors\n", nerrors);
> > +	exit(nerrors == 0 ? 0 : 1);
> > +}
> > +ERROR
> > +{
> > +	exit(1);
> > +}
> > diff --git a/test/unittest/operators/tst.str_comparison-basic.d b/test/unittest/operators/tst.str_comparison-basic.d
> > index 344a2cf0..1e13c70f 100644
> > --- a/test/unittest/operators/tst.str_comparison-basic.d
> > +++ b/test/unittest/operators/tst.str_comparison-basic.d
> > @@ -42,29 +42,6 @@ BEGIN
> >   	nerrors += (s3 >= s2 ? 0 : 1);
> >   	nerrors += (s3 >  s2 ? 0 : 1);
> > -	s2 = NULL;
> > -	nerrors += (s3 <= s2 ? 1 : 0);
> > -	nerrors += (s3 <  s2 ? 1 : 0);
> > -	nerrors += (s3 == s2 ? 1 : 0);
> > -	nerrors += (s3 != s2 ? 0 : 1);
> > -	nerrors += (s3 >= s2 ? 0 : 0);
> > -	nerrors += (s3 >  s2 ? 0 : 0);
> > -
> > -	nerrors += (s2 <= s3 ? 0 : 1);
> > -	nerrors += (s2 <  s3 ? 0 : 1);
> > -	nerrors += (s2 == s3 ? 1 : 0);
> > -	nerrors += (s2 != s3 ? 0 : 1);
> > -	nerrors += (s2 >= s3 ? 1 : 0);
> > -	nerrors += (s2 >  s3 ? 1 : 0);
> > -
> > -	s3 = NULL;
> > -	nerrors += (s2 <= s3 ? 0 : 1);
> > -	nerrors += (s2 <  s3 ? 1 : 0);
> > -	nerrors += (s2 == s3 ? 0 : 1);
> > -	nerrors += (s2 != s3 ? 1 : 0);
> > -	nerrors += (s2 >= s3 ? 0 : 1);
> > -	nerrors += (s2 >  s3 ? 1 : 0);
> > -
> >   	printf("%d errors\n", nerrors);
> >   	exit(nerrors == 0 ? 0 : 1);
> >   }
> > -- 2.37.2 _______________________________________________ DTrace-devel
> > mailing list DTrace-devel at oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel