| Message ID | 20240523124541.7dd4cca9@gandalf.local.home (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 53af1a4b6a55b3910a253fce7a0893e6d51952be |
| Headers | show |
| Series | tracing/selftests: Run the ownership test twice | expand |
On Thu, 23 May 2024 12:45:41 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > A regression happened where running the ownership test passes on the first > iteration but fails running it a second time. This was caught and fixed, > but a later change brought it back. The regression was missed because the > automated tests only run the tests once per boot. > > Change the ownership test to iterate through the tests twice, as this will > catch the regression with a single run. > Looks good to me. Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Thanks! > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > .../ftrace/test.d/00basic/test_ownership.tc | 34 +++++++++++-------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > index c45094d1e1d2..71e43a92352a 100644 > --- a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > @@ -83,32 +83,38 @@ run_tests() { > done > } > > -mount -o remount,"$new_options" . > +# Run the tests twice as leftovers can cause issues > +for loop in 1 2 ; do > > -run_tests > + echo "Running iteration $loop" > > -mount -o remount,"$mount_options" . > + mount -o remount,"$new_options" . > > -for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > - test "$d" $original_group > -done > + run_tests > + > + mount -o remount,"$mount_options" . > + > + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > + test "$d" $original_group > + done > > # check instances as well > > -chgrp $other_group instances > + chgrp $other_group instances > > -instance="$(mktemp -u test-XXXXXX)" > + instance="$(mktemp -u test-XXXXXX)" > > -mkdir instances/$instance > + mkdir instances/$instance > > -cd instances/$instance > + cd instances/$instance > > -run_tests > + run_tests > > -cd ../.. > + cd ../.. > > -rmdir instances/$instance > + rmdir instances/$instance > > -chgrp $original_group instances > + chgrp $original_group instances > +done > > exit 0 > -- > 2.43.0 > >
Shuah, Can you take this through your tree? -- Steve On Thu, 23 May 2024 12:45:41 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > A regression happened where running the ownership test passes on the first > iteration but fails running it a second time. This was caught and fixed, > but a later change brought it back. The regression was missed because the > automated tests only run the tests once per boot. > > Change the ownership test to iterate through the tests twice, as this will > catch the regression with a single run. > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > .../ftrace/test.d/00basic/test_ownership.tc | 34 +++++++++++-------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > index c45094d1e1d2..71e43a92352a 100644 > --- a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > @@ -83,32 +83,38 @@ run_tests() { > done > } > > -mount -o remount,"$new_options" . > +# Run the tests twice as leftovers can cause issues > +for loop in 1 2 ; do > > -run_tests > + echo "Running iteration $loop" > > -mount -o remount,"$mount_options" . > + mount -o remount,"$new_options" . > > -for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > - test "$d" $original_group > -done > + run_tests > + > + mount -o remount,"$mount_options" . > + > + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > + test "$d" $original_group > + done > > # check instances as well > > -chgrp $other_group instances > + chgrp $other_group instances > > -instance="$(mktemp -u test-XXXXXX)" > + instance="$(mktemp -u test-XXXXXX)" > > -mkdir instances/$instance > + mkdir instances/$instance > > -cd instances/$instance > + cd instances/$instance > > -run_tests > + run_tests > > -cd ../.. > + cd ../.. > > -rmdir instances/$instance > + rmdir instances/$instance > > -chgrp $original_group instances > + chgrp $original_group instances > +done > > exit 0
On Fri, 14 Jun 2024 12:41:51 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > Shuah, > > Can you take this through your tree? Ping! -- Steve > > -- Steve > > > On Thu, 23 May 2024 12:45:41 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > > > A regression happened where running the ownership test passes on the first > > iteration but fails running it a second time. This was caught and fixed, > > but a later change brought it back. The regression was missed because the > > automated tests only run the tests once per boot. > > > > Change the ownership test to iterate through the tests twice, as this will > > catch the regression with a single run. > > > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > > --- > > .../ftrace/test.d/00basic/test_ownership.tc | 34 +++++++++++-------- > > 1 file changed, 20 insertions(+), 14 deletions(-) > > > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > index c45094d1e1d2..71e43a92352a 100644 > > --- a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > @@ -83,32 +83,38 @@ run_tests() { > > done > > } > > > > -mount -o remount,"$new_options" . > > +# Run the tests twice as leftovers can cause issues > > +for loop in 1 2 ; do > > > > -run_tests > > + echo "Running iteration $loop" > > > > -mount -o remount,"$mount_options" . > > + mount -o remount,"$new_options" . > > > > -for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > > - test "$d" $original_group > > -done > > + run_tests > > + > > + mount -o remount,"$mount_options" . > > + > > + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do > > + test "$d" $original_group > > + done > > > > # check instances as well > > > > -chgrp $other_group instances > > + chgrp $other_group instances > > > > -instance="$(mktemp -u test-XXXXXX)" > > + instance="$(mktemp -u test-XXXXXX)" > > > > -mkdir instances/$instance > > + mkdir instances/$instance > > > > -cd instances/$instance > > + cd instances/$instance > > > > -run_tests > > + run_tests > > > > -cd ../.. > > + cd ../.. > > > > -rmdir instances/$instance > > + rmdir instances/$instance > > > > -chgrp $original_group instances > > + chgrp $original_group instances > > +done > > > > exit 0 >
On 8/7/24 14:29, Steven Rostedt wrote: > On Fri, 14 Jun 2024 12:41:51 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > >> Shuah, >> >> Can you take this through your tree? > > Ping! > > -- Steve > Sorry for the delay. Applied it o linux-kselftest next for Linux 6.12-rc1. thanks, -- Shuah
diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc index c45094d1e1d2..71e43a92352a 100644 --- a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc @@ -83,32 +83,38 @@ run_tests() { done } -mount -o remount,"$new_options" . +# Run the tests twice as leftovers can cause issues +for loop in 1 2 ; do -run_tests + echo "Running iteration $loop" -mount -o remount,"$mount_options" . + mount -o remount,"$new_options" . -for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do - test "$d" $original_group -done + run_tests + + mount -o remount,"$mount_options" . + + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do + test "$d" $original_group + done # check instances as well -chgrp $other_group instances + chgrp $other_group instances -instance="$(mktemp -u test-XXXXXX)" + instance="$(mktemp -u test-XXXXXX)" -mkdir instances/$instance + mkdir instances/$instance -cd instances/$instance + cd instances/$instance -run_tests + run_tests -cd ../.. + cd ../.. -rmdir instances/$instance + rmdir instances/$instance -chgrp $original_group instances + chgrp $original_group instances +done exit 0