[PATCH 0/2] perf test: Add --skip-name to skip tests by name

[PATCH 0/2] perf test: Add --skip-name to skip tests by name

[James Clark]

We've had to update skip list numbers a few times in our CI, so I think
it's useful to be able to skip tests by name as well.

There are some decisions to make like whether we could put test numbers
into the tests themselves, or re-use the --skip argument for names etc.
But I think the --skip-name in these patches is the simplest and easiest
to use compromise.

I'm sure test names will change sometimes, but it's much rarer than
adding a new test or compiling with a different config which can change
the numbers. No matches resulting in an error means that the same CI
command on different architectures could cause a fatal error, but I
can't see a real use case where this would be an issue because skip
lists are arch and device dependent.

Signed-off-by: James Clark <james.clark@linaro.org>
---
James Clark (2):
      perf test: Factor out the --skip check and its output
      perf test: Add --skip-name to skip tests by name

 tools/perf/Documentation/perf-test.txt |   4 +
 tools/perf/tests/builtin-test.c        | 146 ++++++++++++++++++++++++---------
 2 files changed, 110 insertions(+), 40 deletions(-)
---
base-commit: 1c90f0a3c8a1c4e38f9edd97af9e20848805a7fb
change-id: 20261022-james-perf-test-skip-c923ea7198fe

Best regards,
--  
James Clark <james.clark@linaro.org>

[PATCH 1/2] perf test: Factor out the --skip check and its output

[James Clark]

__cmd_test() checks the --skip list and prints the "Skip (user
override)" line and its junit entry inline, and
perf_test__shares_mlock_budget() repeats the check, with the list passed
to both from cmd_test().

Make the list static, like the other options, check it in
perf_test__skip_suite() and move the output to
print_suite_user_override(), so that the --skip-name option added next
can hook into the same places.

No functional changes intended.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/tests/builtin-test.c | 83 +++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 880eece0bc12..cd0fc3dedc3e 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -68,6 +68,8 @@ static bool list_requisites;
 static unsigned int runs_per_test = 1;
 /* Number of lines to include in failure snippet. */
 static unsigned int failure_snippet_lines = 10;
+/* Numbers of the test suites to skip, from --skip. */
+static struct intlist *skiplist;
 /*
  * The tests mapping ring buffers take turns using their share of
  * kernel.perf_event_mlock_kb via this mutex, shared with the forked test
@@ -342,6 +344,11 @@ static bool test_exclusive(const struct test_suite *t, int test_case)
 	return t->test_cases[test_case].exclusive;
 }
 
+static bool perf_test__skip_suite(int suite_num)
+{
+	return intlist__find(skiplist, suite_num + 1) != NULL;
+}
+
 static bool perf_test__matches(const char *desc, int suite_num, int argc, const char *argv[])
 {
 	int i;
@@ -1358,8 +1365,33 @@ static void print_tests_summary(void)
 	strbuf_release(&summary_failed_tests_buf);
 }
 
-static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
-		      struct intlist *skiplist)
+static void print_suite_user_override(struct test_suite *t, int curr_suite, int width)
+{
+	int pad_width = get_max_desc_width(width);
+	int term_width = get_term_width();
+
+	pr_info("%3d: %-*.*s:", curr_suite + 1, pad_width, term_width,
+		test_description(t, -1));
+	color_fprintf(debug_file(), PERF_COLOR_YELLOW, " Skip (user override)\n");
+	summary_tests_skipped++;
+	if (junit_filename) {
+		char *escaped_class = xml_escape(test_description(t, -1));
+		char *escaped_test = xml_escape("override");
+		char *escaped_reason = xml_escape("user override");
+
+		strbuf_addf(&junit_xml_buf,
+			    "    <testcase classname=\"%s\" name=\"%s\" time=\"0.000\">\n",
+			    escaped_class, escaped_test);
+		strbuf_addf(&junit_xml_buf, "      <skipped message=\"%s\"/>\n",
+			    escaped_reason);
+		strbuf_addstr(&junit_xml_buf, "    </testcase>\n");
+		free(escaped_reason);
+		free(escaped_test);
+		free(escaped_class);
+	}
+}
+
+static int __cmd_test(struct test_suite **suites, int argc, const char *argv[])
 {
 	static int width = 0;
 	int err = 0;
@@ -1434,37 +1466,9 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
 				suite_matched = true;
 			}
 
-			if (intlist__find(skiplist, curr_suite + 1)) {
-				if (pass == 1) {
-					int pad_width = get_max_desc_width(width);
-					int term_width = get_term_width();
-
-					pr_info("%3d: %-*.*s:", curr_suite + 1,
-						pad_width, term_width,
-						test_description(*t, -1));
-					color_fprintf(debug_file(), PERF_COLOR_YELLOW,
-						      " Skip (user override)\n");
-					summary_tests_skipped++;
-					if (junit_filename) {
-						char *escaped_class =
-							xml_escape((const char *)
-								   test_description(*t, -1));
-						char *escaped_test = xml_escape("override");
-						char *escaped_reason =
-							xml_escape("user override");
-
-						strbuf_addf(&junit_xml_buf,
-							"    <testcase classname=\"%s\" name=\"%s\" time=\"0.000\">\n",
-							escaped_class, escaped_test);
-						strbuf_addf(&junit_xml_buf,
-							"      <skipped message=\"%s\"/>\n",
-							escaped_reason);
-						strbuf_addstr(&junit_xml_buf, "    </testcase>\n");
-						free(escaped_reason);
-						free(escaped_test);
-						free(escaped_class);
-					}
-				}
+			if (perf_test__skip_suite(curr_suite)) {
+				if (pass == 1)
+					print_suite_user_override(*t, curr_suite, width);
 				continue;
 			}
 
@@ -1817,7 +1821,7 @@ static struct test_suite **build_suites(void)
 }
 
 static bool perf_test__shares_mlock_budget(struct test_suite **suites, int argc,
-					   const char *argv[], struct intlist *skiplist)
+					   const char *argv[])
 {
 	int sharing = 0;
 
@@ -1827,7 +1831,7 @@ static bool perf_test__shares_mlock_budget(struct test_suite **suites, int argc,
 		int curr_test_case;
 		bool suite_matched;
 
-		if (!t->needs_mlock || intlist__find(skiplist, curr_suite + 1))
+		if (!t->needs_mlock || perf_test__skip_suite(curr_suite))
 			continue;
 
 		suite_matched = perf_test__matches(test_description(t, /*test_case=*/-1),
@@ -1865,7 +1869,7 @@ static bool perf_test__mlock_budget_exhaustible(void)
 }
 
 static void perf_test__setup_mlock_budget(struct test_suite **suites, int argc,
-					  const char *argv[], struct intlist *skiplist)
+					  const char *argv[])
 {
 	/* Nothing can be denied the budget, nothing to take turns for. */
 	if (!perf_test__mlock_budget_exhaustible()) {
@@ -1874,7 +1878,7 @@ static void perf_test__setup_mlock_budget(struct test_suite **suites, int argc,
 	}
 
 	/* Nothing runs at the same time, nothing to share the budget with. */
-	if (sequential || !perf_test__shares_mlock_budget(suites, argc, argv, skiplist))
+	if (sequential || !perf_test__shares_mlock_budget(suites, argc, argv))
 		return;
 
 	mlock_budget = mmap(NULL, sizeof(*mlock_budget), PROT_READ | PROT_WRITE,
@@ -1936,7 +1940,6 @@ int cmd_test(int argc, const char **argv)
 	OPT_END()
 	};
 	const char * const test_subcommands[] = { "list", "requisites", NULL };
-	struct intlist *skiplist = NULL;
 	struct test_suite **suites;
 	int ret;
 
@@ -2010,8 +2013,8 @@ int cmd_test(int argc, const char **argv)
 		intlist__delete(skiplist);
 		return err ? -err : -ENOMEM;
 	}
-	perf_test__setup_mlock_budget(suites, argc, argv, skiplist);
-	ret = __cmd_test(suites, argc, argv, skiplist);
+	perf_test__setup_mlock_budget(suites, argc, argv);
+	ret = __cmd_test(suites, argc, argv);
 	perf_test__teardown_mlock_budget();
 	free(suites);
 	intlist__delete(skiplist);

-- 
2.43.0

[PATCH 2/2] perf test: Add --skip-name to skip tests by name

[James Clark]

--skip takes test numbers which change whenever tests are added or
removed, so a list of tests to skip in CI has to be updated as perf
changes, or it silently skips the wrong tests.

Add --skip-name to skip a test by its name which should be more stable.
The option can be repeated to skip more than one test instead of taking
a comma separated list, as some descriptions contain commas, e.g.:

  $ perf test --skip-name 'Add fd to a fdarray, making it autogrow' \
              --skip-name 'Simple expression parser' 7 32 33
    7: Simple expression parser                : Skip (user override)
   33: Add fd to a fdarray, making it autogrow : Skip (user override)
   32: Filter fds with revents mask in a fdarray : Ok

The match is exact rather than substring so that adding a new test
containing an existing skip substring doesn't get skipped.

A name that matches no test exits with an error so that a renamed test
isn't silently run again:

  $ perf test --skip-name 'Add fd to a fdarray'
  No test matches --skip-name 'Add fd to a fdarray'
  $ echo $?
  234

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/Documentation/perf-test.txt |  4 ++
 tools/perf/tests/builtin-test.c        | 71 ++++++++++++++++++++++++++++++++--
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt
index f66ccd77bfe7..bc3426705a80 100644
--- a/tools/perf/Documentation/perf-test.txt
+++ b/tools/perf/Documentation/perf-test.txt
@@ -57,6 +57,10 @@ OPTIONS
 --skip::
 	Tests to skip (comma separated numeric list).
 
+--skip-name=<name>::
+	Skip the test whose description is exactly <name>, as printed by
+	'perf test list'. Can be provided multiple times.
+
 -v::
 -vv::
 -vvv::
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index cd0fc3dedc3e..433534b8e182 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -42,6 +42,7 @@
 #include "hist.h"
 #include "intlist.h"
 #include "string2.h"
+#include "strlist.h"
 #include "symbol.h"
 #include "tests-scripts.h"
 #include "tests.h"
@@ -70,6 +71,8 @@ static unsigned int runs_per_test = 1;
 static unsigned int failure_snippet_lines = 10;
 /* Numbers of the test suites to skip, from --skip. */
 static struct intlist *skiplist;
+/* Exact descriptions of the tests to skip, from --skip-name. */
+static struct strlist *skip_names;
 /*
  * The tests mapping ring buffers take turns using their share of
  * kernel.perf_event_mlock_kb via this mutex, shared with the forked test
@@ -344,9 +347,12 @@ static bool test_exclusive(const struct test_suite *t, int test_case)
 	return t->test_cases[test_case].exclusive;
 }
 
-static bool perf_test__skip_suite(int suite_num)
+static bool perf_test__skip_suite(const struct test_suite *t, int suite_num)
 {
-	return intlist__find(skiplist, suite_num + 1) != NULL;
+	if (intlist__find(skiplist, suite_num + 1))
+		return true;
+
+	return skip_names && strlist__has_entry(skip_names, test_description(t, -1));
 }
 
 static bool perf_test__matches(const char *desc, int suite_num, int argc, const char *argv[])
@@ -1466,7 +1472,7 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[])
 				suite_matched = true;
 			}
 
-			if (perf_test__skip_suite(curr_suite)) {
+			if (perf_test__skip_suite(*t, curr_suite)) {
 				if (pass == 1)
 					print_suite_user_override(*t, curr_suite, width);
 				continue;
@@ -1510,6 +1516,52 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[])
 	return err;
 }
 
+static int parse_skip_name(const struct option *opt __maybe_unused, const char *str,
+			   int unset)
+{
+	int err;
+
+	/* --no-skip-name clears the list, like --no-skip does. */
+	if (unset) {
+		strlist__delete(skip_names);
+		skip_names = NULL;
+		return 0;
+	}
+
+	if (!skip_names) {
+		skip_names = strlist__new(NULL, NULL);
+		if (!skip_names)
+			return -ENOMEM;
+	}
+	err = strlist__add(skip_names, str);
+
+	/* Asking to skip a test twice is harmless. */
+	return err == -EEXIST ? 0 : err;
+}
+
+/*
+ * A --skip-name that matches nothing is likely a typo or a test that was
+ * renamed, report it rather than silently running the test.
+ */
+static int perf_test__check_skip_names(struct test_suite **suites)
+{
+	struct str_node *pos;
+	int err = 0;
+
+	strlist__for_each_entry(pos, skip_names) {
+		bool found = false;
+
+		for (struct test_suite **t = suites; *t && !found; t++)
+			found = !strcmp(test_description(*t, -1), pos->s);
+
+		if (!found) {
+			pr_err("No test matches --skip-name '%s'\n", pos->s);
+			err = -EINVAL;
+		}
+	}
+	return err;
+}
+
 static int perf_test__list(FILE *fp, struct test_suite **suites, int argc, const char **argv)
 {
 	int curr_suite = 0;
@@ -1831,7 +1883,7 @@ static bool perf_test__shares_mlock_budget(struct test_suite **suites, int argc,
 		int curr_test_case;
 		bool suite_matched;
 
-		if (!t->needs_mlock || perf_test__skip_suite(curr_suite))
+		if (!t->needs_mlock || perf_test__skip_suite(t, curr_suite))
 			continue;
 
 		suite_matched = perf_test__matches(test_description(t, /*test_case=*/-1),
@@ -1918,6 +1970,9 @@ int cmd_test(int argc, const char **argv)
 	bool list_workloads = false;
 	const struct option test_options[] = {
 	OPT_STRING('s', "skip", &skip, "tests", "tests to skip"),
+	OPT_CALLBACK(0, "skip-name", NULL, "name",
+		     "test to skip, by its exact name, can be repeated",
+		     parse_skip_name),
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show symbol address, etc)"),
 	OPT_BOOLEAN('F', "dont-fork", &dont_fork,
@@ -2011,12 +2066,20 @@ int cmd_test(int argc, const char **argv)
 		int err = errno;
 
 		intlist__delete(skiplist);
+		strlist__delete(skip_names);
 		return err ? -err : -ENOMEM;
 	}
+	if (skip_names) {
+		ret = perf_test__check_skip_names(suites);
+		if (ret)
+			goto out;
+	}
 	perf_test__setup_mlock_budget(suites, argc, argv);
 	ret = __cmd_test(suites, argc, argv);
 	perf_test__teardown_mlock_budget();
+out:
 	free(suites);
 	intlist__delete(skiplist);
+	strlist__delete(skip_names);
 	return ret;
 }

-- 
2.43.0

Re: [PATCH 1/2] perf test: Factor out the --skip check and its output

[sashiko-bot]

> __cmd_test() checks the --skip list and prints the "Skip (user
> override)" line and its junit entry inline, and
> perf_test__shares_mlock_budget() repeats the check, with the list passed
> to both from cmd_test().
> 
> Make the list static, like the other options, check it in
> perf_test__skip_suite() and move the output to
> print_suite_user_override(), so that the --skip-name option added next
> can hook into the same places.
> 
> No functional changes intended.
> 
> Signed-off-by: James Clark <james.clark@linaro.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-james-perf-test-skip-v1-0-b6c11bdafd46@linaro.org?part=1

Re: [PATCH 2/2] perf test: Add --skip-name to skip tests by name

[sashiko-bot]

> --skip takes test numbers which change whenever tests are added or
> removed, so a list of tests to skip in CI has to be updated as perf
> changes, or it silently skips the wrong tests.
> 
> Add --skip-name to skip a test by its name which should be more stable.
> The option can be repeated to skip more than one test instead of taking
> a comma separated list, as some descriptions contain commas, e.g.:
> 
>   $ perf test --skip-name 'Add fd to a fdarray, making it autogrow' \
>               --skip-name 'Simple expression parser' 7 32 33
>     7: Simple expression parser                : Skip (user override)
>    33: Add fd to a fdarray, making it autogrow : Skip (user override)
>    32: Filter fds with revents mask in a fdarray : Ok
> 
> The match is exact rather than substring so that adding a new test
> containing an existing skip substring doesn't get skipped.
> 
> A name that matches no test exits with an error so that a renamed test
> isn't silently run again:
> 
>   $ perf test --skip-name 'Add fd to a fdarray'
>   No test matches --skip-name 'Add fd to a fdarray'
>   $ echo $?
>   234
> 
> Signed-off-by: James Clark <james.clark@linaro.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-james-perf-test-skip-v1-0-b6c11bdafd46@linaro.org?part=2