From: Kris Van Hees <kris.van.hees@oracle.com>
To: eugene.loh@oracle.com
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [PATCH v2] test: Make tests more resilient to different prid widths
Date: Wed, 19 Mar 2025 13:46:02 -0400 [thread overview]
Message-ID: <Z9sC2i3zJRbT0nzG@oracle.com> (raw)
In-Reply-To: <20250319171525.1877-1-eugene.loh@oracle.com>
On Wed, Mar 19, 2025 at 01:15:25PM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
>
> Various tests convert run-dependent values -- like PIDs and probe IDs
> -- to run-independent strings before checking against their .r results
> files. But the conversions could be remarkably sensitive to the width
> of probe IDs. E.g., some conversions assumed probe IDs were flush with
> the beginning of the line, but if they were narrower they were preceded
> by white space and were not detected. E.g., this happened in recent fbt
> work, where probe IDs for fbt probes became much smaller in value.
>
> Also, these conversions were being carried out by a hodgepodge of scripts
> -- sed, awk, and grep; some using run-independent strings like "NNN" or
> "XXXX" instead of more informative "PID" and "PRID" strings; some
> incorrectly using "PID" for PRIDs, etc.
>
> Replace these .r.p postprocessing scripts with a single script that is
> more resilient to PRID widths and is commented.
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
> test/unittest/usdt/convert_PID_and_PRID.awk | 20 +++++++++++++++++
> test/unittest/usdt/err.argmap-null.r | 2 +-
> test/unittest/usdt/err.argmap-null.r.p | 3 +--
> test/unittest/usdt/tst.dlclose1.r | 8 +++----
> test/unittest/usdt/tst.dlclose1.r.p | 13 +----------
> test/unittest/usdt/tst.enable_pid.r | 22 +++++++++----------
> test/unittest/usdt/tst.enable_pid.r.p | 8 +------
> test/unittest/usdt/tst.exec-dof-replacement.r | 2 +-
> .../usdt/tst.exec-dof-replacement.r.p | 3 +--
> .../usdt/tst.multiprov-dupprobe-fire.r.p | 3 +--
> test/unittest/usdt/tst.multiprov-dupprobe.r.p | 6 +----
> test/unittest/usdt/tst.multiprovider-fire.r.p | 3 +--
> test/unittest/usdt/tst.multiprovider.r.p | 6 +----
> 13 files changed, 44 insertions(+), 55 deletions(-)
> create mode 100755 test/unittest/usdt/convert_PID_and_PRID.awk
> mode change 100755 => 120000 test/unittest/usdt/err.argmap-null.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.dlclose1.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.enable_pid.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.exec-dof-replacement.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.multiprov-dupprobe.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.multiprovider-fire.r.p
> mode change 100755 => 120000 test/unittest/usdt/tst.multiprovider.r.p
>
> diff --git a/test/unittest/usdt/convert_PID_and_PRID.awk b/test/unittest/usdt/convert_PID_and_PRID.awk
> new file mode 100755
> index 000000000..1dbb31301
> --- /dev/null
> +++ b/test/unittest/usdt/convert_PID_and_PRID.awk
> @@ -0,0 +1,20 @@
> +#!/usr/bin/gawk -f
> +
> +# ignore the banner
> +/^ *ID *PROVIDER *MODULE *FUNCTION *NAME *$/ { next; }
I think that using / +/ for the whitespace instances might be better,
or perhaps even /[ \t]+/ if we want to guard against future use of tabs.
But at a minimum / +/ seems most prudent.
> +
> +# process other lines
> +{
> + # convert run-dependent PID values to "PID"
> + $0 = gensub("prov([abc]?)[0-9]+", "prov\\1PID", "g");
> + sub("pid [0-9]+", "pid PID");
First arg of each function is a regexp, so should be specified as /.../ rather
than "...".
> +
> + # convert run-dependent probe ID values to "PRID"
> + sub("^ *[0-9]+", "PRID");
Same.
> +
> + # squash blanks
> + gsub(" +", " ");
Same.
> +
> + # print
> + print;
> +}
> diff --git a/test/unittest/usdt/err.argmap-null.r b/test/unittest/usdt/err.argmap-null.r
> index 215475e39..97b1850de 100644
> --- a/test/unittest/usdt/err.argmap-null.r
> +++ b/test/unittest/usdt/err.argmap-null.r
> @@ -1,2 +1,2 @@
> -- @@stderr --
> -dtrace: failed to compile script test/unittest/usdt/err.argmap-null.d: line 24: index 0 is out of range for test_provXXXX:::place4 args[ ]
> +dtrace: failed to compile script test/unittest/usdt/err.argmap-null.d: line 24: index 0 is out of range for test_provPID:::place4 args[ ]
> diff --git a/test/unittest/usdt/err.argmap-null.r.p b/test/unittest/usdt/err.argmap-null.r.p
> deleted file mode 100755
> index c575983ad..000000000
> --- a/test/unittest/usdt/err.argmap-null.r.p
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#!/bin/sed -rf
> -s,test_prov[0-9]*,test_provXXXX,g; s,^ *[0-9]+, XX,g;
> diff --git a/test/unittest/usdt/err.argmap-null.r.p b/test/unittest/usdt/err.argmap-null.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/err.argmap-null.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.dlclose1.r b/test/unittest/usdt/tst.dlclose1.r
> index 7873cb51f..70bb50d76 100644
> --- a/test/unittest/usdt/tst.dlclose1.r
> +++ b/test/unittest/usdt/tst.dlclose1.r
> @@ -1,6 +1,4 @@
> -started pid NNN
> - ID PROVIDER MODULE FUNCTION NAME
> -NNN test_provNNN livelib.so go go
> - ID PROVIDER MODULE FUNCTION NAME
> +started pid PID
> +PRID test_provPID livelib.so go go
> -- @@stderr --
> -dtrace: failed to match test_provNNN:::: No probe matches description
> +dtrace: failed to match test_provPID:::: No probe matches description
> diff --git a/test/unittest/usdt/tst.dlclose1.r.p b/test/unittest/usdt/tst.dlclose1.r.p
> deleted file mode 100755
> index 85725f3bb..000000000
> --- a/test/unittest/usdt/tst.dlclose1.r.p
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -#!/usr/bin/gawk -f
> -{
> - # ignore the specific probe ID or process ID
> - # (the script ensures the process ID is consistent)
> - gsub(/[0-9]+/, "NNN");
> -
> - # ignore the numbers of spaces for alignment
> - # (they depend on the ID widths)
> - gsub(/ +/, " ");
> -
> - print;
> -}
> diff --git a/test/unittest/usdt/tst.dlclose1.r.p b/test/unittest/usdt/tst.dlclose1.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.dlclose1.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.enable_pid.r b/test/unittest/usdt/tst.enable_pid.r
> index 675fcdd6f..9241202d7 100644
> --- a/test/unittest/usdt/tst.enable_pid.r
> +++ b/test/unittest/usdt/tst.enable_pid.r
> @@ -1,14 +1,14 @@
> - FUNCTION:NAME
> - :tick-1s
> + FUNCTION:NAME
> + :tick-1s
>
> - FUNCTION:NAME
> - :tick-1s
> + FUNCTION:NAME
> + :tick-1s
>
> - FUNCTION:NAME
> - :tick-1s
> + FUNCTION:NAME
> + :tick-1s
>
> - FUNCTION:NAME
> - :tick-1s
> + FUNCTION:NAME
> + :tick-1s
>
> done
> ========== out 1
> @@ -39,7 +39,7 @@ is not enabled
> === epoch ===
> success
> -- @@stderr --
> -dtrace: description 'test_provNNN:::go ' matched 1 probe
> -dtrace: description 'test_provNNN:::go ' matched 2 probes
> -dtrace: description 'test_provNNN:::go ' matched 2 probes
> +dtrace: description 'test_provPID:::go ' matched 1 probe
> +dtrace: description 'test_provPID:::go ' matched 2 probes
> +dtrace: description 'test_provPID:::go ' matched 2 probes
> dtrace: description 'test_prov*:::go ' matched 3 probes
> diff --git a/test/unittest/usdt/tst.enable_pid.r.p b/test/unittest/usdt/tst.enable_pid.r.p
> deleted file mode 100755
> index baf9d2a90..000000000
> --- a/test/unittest/usdt/tst.enable_pid.r.p
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#!/usr/bin/awk -f
> -{
> - # ignore the specific process ID
> - gsub(/test_prov[0-9]+/, "test_provNNN");
> -
> - print;
> -}
> diff --git a/test/unittest/usdt/tst.enable_pid.r.p b/test/unittest/usdt/tst.enable_pid.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.enable_pid.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.exec-dof-replacement.r b/test/unittest/usdt/tst.exec-dof-replacement.r
> index 7547f85e5..226ab7c8a 100644
> --- a/test/unittest/usdt/tst.exec-dof-replacement.r
> +++ b/test/unittest/usdt/tst.exec-dof-replacement.r
> @@ -1 +1 @@
> -PID test_prov test2 main succeeded
> +PRID test_provPID test2 main succeeded
> diff --git a/test/unittest/usdt/tst.exec-dof-replacement.r.p b/test/unittest/usdt/tst.exec-dof-replacement.r.p
> deleted file mode 100755
> index 1a5871f73..000000000
> --- a/test/unittest/usdt/tst.exec-dof-replacement.r.p
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#!/bin/sh
> -grep -v '^ *ID' | sed 's,^[0-9]*,PID,; s,prov[0-9]*,prov,g; s, *, ,g'
> diff --git a/test/unittest/usdt/tst.exec-dof-replacement.r.p b/test/unittest/usdt/tst.exec-dof-replacement.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.exec-dof-replacement.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p b/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p
> deleted file mode 100755
> index bdbce0189..000000000
> --- a/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#!/bin/sh
> -sed 's,prov\(.\)[0-9]*,prov\1PID,; s, *, ,g'
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p b/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprov-dupprobe-fire.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe.r.p b/test/unittest/usdt/tst.multiprov-dupprobe.r.p
> deleted file mode 100755
> index 5d11db2d4..000000000
> --- a/test/unittest/usdt/tst.multiprov-dupprobe.r.p
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -#!/bin/sh
> -
> -# Remove banner.
> -# Replace numerical values with generic PRID and PID labels.
> -grep -v '^ *ID' | sed 's,^[0-9][0-9]*,PRID,; s,prov\(.\)[0-9]*,prov\1PID,; s, *, ,g'
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe.r.p b/test/unittest/usdt/tst.multiprov-dupprobe.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprov-dupprobe.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprovider-fire.r.p b/test/unittest/usdt/tst.multiprovider-fire.r.p
> deleted file mode 100755
> index bdbce0189..000000000
> --- a/test/unittest/usdt/tst.multiprovider-fire.r.p
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -#!/bin/sh
> -sed 's,prov\(.\)[0-9]*,prov\1PID,; s, *, ,g'
> diff --git a/test/unittest/usdt/tst.multiprovider-fire.r.p b/test/unittest/usdt/tst.multiprovider-fire.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprovider-fire.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprovider.r.p b/test/unittest/usdt/tst.multiprovider.r.p
> deleted file mode 100755
> index 5d11db2d4..000000000
> --- a/test/unittest/usdt/tst.multiprovider.r.p
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -#!/bin/sh
> -
> -# Remove banner.
> -# Replace numerical values with generic PRID and PID labels.
> -grep -v '^ *ID' | sed 's,^[0-9][0-9]*,PRID,; s,prov\(.\)[0-9]*,prov\1PID,; s, *, ,g'
> diff --git a/test/unittest/usdt/tst.multiprovider.r.p b/test/unittest/usdt/tst.multiprovider.r.p
> new file mode 120000
> index 000000000..11a06e058
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprovider.r.p
> @@ -0,0 +1 @@
> +convert_PID_and_PRID.awk
> \ No newline at end of file
> --
> 2.43.5
>
prev parent reply other threads:[~2025-03-19 17:46 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-19 17:15 [PATCH v2] test: Make tests more resilient to different prid widths eugene.loh
2025-03-19 17:46 ` Kris Van Hees [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z9sC2i3zJRbT0nzG@oracle.com \
--to=kris.van.hees@oracle.com \
--cc=dtrace-devel@oss.oracle.com \
--cc=dtrace@lists.linux.dev \
--cc=eugene.loh@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.