[DTrace-devel] [PATCH] tests: Use distinct trigger for copyin* tests
Kris Van Hees
kris.van.hees at oracle.com
Fri Jan 13 21:01:58 UTC 2023
In principle, I am OK with these changes.
But... by making these changes you got rid of additional testing that was
being done and that actually is important. So, yes, making the tests more
true unit tests is a good thing, but then we need to add additional tests
to capture other things that are being exercise, like:
- being able to cast the return value of copyin() to string
- being able to use indexing on the cast value
- being able to use indexing on the return value of copyinstr()
And some additional testing that we are not doing yet (I think) should be
added also:
- verifying that casting the result of copyin() to string results
in a copy operation rather than a pointer assignment
The first set definitely should be part of this patch or be a 2nd patch that
is put in a patch set with this one, to ensure that we do not lose testing
coverage by the proposed simplification.
On Fri, Jan 13, 2023 at 03:27:18PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> Many copyin* tests implement a trigger by running system("echo foo")
> in the BEGIN probe. This requires a destructive action, which is okay
> but does make these "unit tests" rely on more features.
>
> The main problem, however, is that the check that the corresponding
> write:entry probe has fired on the correct process becomes very weak
> -- e.g., just checking that a particular char is '-'. And in the case
> of copyinto, this check is unnecessarily complicated. Finally, if
> such a process is not found, the test would hang.
>
> Use @@trigger. E.g., delaydie will quickly terminate, emiting a simple
> message, which is all we need for these tests.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> test/unittest/funcs/copyin/tst.copyin.d | 13 ++++---------
> test/unittest/funcs/copyin/tst.copyin.r | 6 +++---
> .../copyinstr/tst.copyinstr-low-maxsize.d | 13 ++++---------
> .../copyinstr/tst.copyinstr-low-maxsize.r | 5 +++--
> .../funcs/copyinstr/tst.copyinstr-no-maxsize.d | 13 ++++---------
> .../funcs/copyinstr/tst.copyinstr-no-maxsize.r | 5 +++--
> test/unittest/funcs/copyinstr/tst.copyinstr.d | 13 ++++---------
> test/unittest/funcs/copyinstr/tst.copyinstr.r | 6 +++---
> test/unittest/funcs/copyinto/tst.copyinto.d | 18 +++---------------
> test/unittest/funcs/copyinto/tst.copyinto.r | 6 +++---
> 10 files changed, 34 insertions(+), 64 deletions(-)
>
> diff --git a/test/unittest/funcs/copyin/tst.copyin.d b/test/unittest/funcs/copyin/tst.copyin.d
> index 11d52a5a..d76068f6 100644
> --- a/test/unittest/funcs/copyin/tst.copyin.d
> +++ b/test/unittest/funcs/copyin/tst.copyin.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> @@ -11,19 +11,14 @@
> * SECTION: Actions and Subroutines/copyin()
> * User Process Tracing/copyin() and copyinstr() Subroutines
> */
> +/* @@trigger: delaydie */
>
> #pragma D option quiet
> -#pragma D option destructive
> -
> -BEGIN
> -{
> - system("echo dtrace-copyin-test");
> -}
>
> syscall::write:entry
> -/(s = (string)copyin(arg1, 32))[6] == '-'/
> +/pid == $target/
> {
> - printf("'%s'", s);
> + printf("'%s'", (string)copyin(arg1, 32));
> exit(0);
> }
>
> diff --git a/test/unittest/funcs/copyin/tst.copyin.r b/test/unittest/funcs/copyin/tst.copyin.r
> index 05d2c96c..16e2bbcc 100644
> --- a/test/unittest/funcs/copyin/tst.copyin.r
> +++ b/test/unittest/funcs/copyin/tst.copyin.r
> @@ -1,3 +1,3 @@
> -dtrace-copyin-test
> -'dtrace-copyin-test
> -'
> +'Delay in ns needed in delay env '
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.d b/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.d
> index b7502c27..77ac7986 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.d
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> @@ -11,19 +11,14 @@
> * SECTION: Actions and Subroutines/copyinstr()
> * User Process Tracing/copyin() and copyinstr() Subroutines
> */
> +/* @@trigger: delaydie */
>
> #pragma D option quiet
> -#pragma D option destructive
> -
> -BEGIN
> -{
> - system("echo dtrace-copyinstr-test");
> -}
>
> syscall::write:entry
> -/(s = copyinstr(arg1, 8))[6] == '-'/
> +/pid == $target/
> {
> - printf("'%s'", s);
> + printf("'%s'", copyinstr(arg1, 8));
> exit(0);
> }
>
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.r b/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.r
> index 45ca768e..19f2a5bd 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.r
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-low-maxsize.r
> @@ -1,2 +1,3 @@
> -dtrace-copyinstr-test
> -'dtrace-'
> +'Delay i'
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d b/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d
> index 448fa7c5..fc0b5687 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> @@ -11,19 +11,14 @@
> * SECTION: Actions and Subroutines/copyinstr()
> * User Process Tracing/copyin() and copyinstr() Subroutines
> */
> +/* @@trigger: delaydie */
>
> #pragma D option quiet
> -#pragma D option destructive
> -
> -BEGIN
> -{
> - system("echo dtrace-copyinstr-test");
> -}
>
> syscall::write:entry
> -/(s = copyinstr(arg1))[6] == '-'/
> +/pid == $target/
> {
> - printf("'%s'", s);
> + printf("'%s'", copyinstr(arg1));
> exit(0);
> }
>
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.r b/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.r
> index 2be678ab..eeffe362 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.r
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr-no-maxsize.r
> @@ -1,3 +1,4 @@
> -dtrace-copyinstr-test
> -'dtrace-copyinstr-test
> +'Delay in ns needed in delay env var.
> '
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr.d b/test/unittest/funcs/copyinstr/tst.copyinstr.d
> index 09f382b3..68ca7b65 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr.d
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> @@ -11,19 +11,14 @@
> * SECTION: Actions and Subroutines/copyinstr()
> * User Process Tracing/copyin() and copyinstr() Subroutines
> */
> +/* @@trigger: delaydie */
>
> #pragma D option quiet
> -#pragma D option destructive
> -
> -BEGIN
> -{
> - system("echo dtrace-copyinstr-test");
> -}
>
> syscall::write:entry
> -/(s = copyinstr(arg1, 32))[6] == '-'/
> +/pid == $target/
> {
> - printf("'%s'", s);
> + printf("'%s'", copyinstr(arg1, 32));
> exit(0);
> }
>
> diff --git a/test/unittest/funcs/copyinstr/tst.copyinstr.r b/test/unittest/funcs/copyinstr/tst.copyinstr.r
> index 2be678ab..7e80ec2d 100644
> --- a/test/unittest/funcs/copyinstr/tst.copyinstr.r
> +++ b/test/unittest/funcs/copyinstr/tst.copyinstr.r
> @@ -1,3 +1,3 @@
> -dtrace-copyinstr-test
> -'dtrace-copyinstr-test
> -'
> +'Delay in ns needed in delay env'
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> diff --git a/test/unittest/funcs/copyinto/tst.copyinto.d b/test/unittest/funcs/copyinto/tst.copyinto.d
> index b0ec658b..b4c7447f 100644
> --- a/test/unittest/funcs/copyinto/tst.copyinto.d
> +++ b/test/unittest/funcs/copyinto/tst.copyinto.d
> @@ -1,6 +1,6 @@
> /*
> * Oracle Linux DTrace.
> - * Copyright (c) 2006, 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2006, 2023, 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.
> */
> @@ -10,24 +10,12 @@
> *
> * SECTION: Actions and Subroutines/copyinto()
> */
> +/* @@trigger: delaydie */
>
> #pragma D option quiet
> -#pragma D option destructive
> -
> -BEGIN
> -{
> - system("echo dtrace-copyinto-test");
> -}
> -
> -syscall::write:entry
> -{
> - ptr = (char *)alloca(32);
> - copyinto(arg1, 32, ptr);
> - ok = ptr[6] == '-';
> -}
>
> syscall::write:entry
> -/ok/
> +/pid == $target/
> {
> ptr = (char *)alloca(32);
> copyinto(arg1, 32, ptr);
> diff --git a/test/unittest/funcs/copyinto/tst.copyinto.r b/test/unittest/funcs/copyinto/tst.copyinto.r
> index 44dd1c87..16e2bbcc 100644
> --- a/test/unittest/funcs/copyinto/tst.copyinto.r
> +++ b/test/unittest/funcs/copyinto/tst.copyinto.r
> @@ -1,3 +1,3 @@
> -dtrace-copyinto-test
> -'dtrace-copyinto-test
> -'
> +'Delay in ns needed in delay env '
> +-- @@stderr --
> +Delay in ns needed in delay env var.
> --
> 2.18.4
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list