All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
@ 2015-12-07 18:52 Michael Petlan
  2015-12-15 12:03 ` Jiri Olsa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Petlan @ 2015-12-07 18:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa; +Cc: linux-perf-users

In order to be able to run the bash/subcommand-based testsuite directly
from perf test a new perf-test entry has been created for it.

The driver (suite.c) runs all the tests  that it finds in the testsuite
directory: tools/perf/testsuite/   (rather ./testsuite as it expects it
it being run from the perf's directory).

In case the testsuite is not found, the testcase is skipped. If this is
not the desired behaviour, some packaging will be needed.

Usage:
	./perf test suite

or
	./perf test suite -v

(for the verbose output)

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   4 ++
 tools/perf/tests/suite.c        | 143 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/suite.h        |  30 +++++++++
 tools/perf/tests/tests.h        |   1 +
 5 files changed, 179 insertions(+)
 create mode 100644 tools/perf/tests/suite.c
 create mode 100644 tools/perf/tests/suite.h

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 0ff8a97..3bf23a6 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -34,6 +34,7 @@ perf-y += thread-map.o
 perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o llvm-src-prologue.o
 perf-y += bpf.o
 perf-y += topology.o
+perf-y += suite.o
 
 $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
 	$(call rule_mkdir)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 2b1ade1..6a3aa60 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -180,6 +180,10 @@ static struct test generic_tests[] = {
 		},
 	},
 	{
+		.desc = "Testsuite",
+		.func = test__suite,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
new file mode 100644
index 0000000..b79f66b
--- /dev/null
+++ b/tools/perf/tests/suite.c
@@ -0,0 +1,143 @@
+#include "suite.h"
+
+#define TESTSUITE_ROOT "./testsuite/"
+
+/* globals */
+int fatal_occured = 0;
+char *cwd;
+
+
+/* runs a shell script */
+int _run_shell(const char *script)
+{
+	int ret;
+	char *cmd = malloc(strlen(script) + 3 * sizeof(char));
+	strcpy(cmd, "./");
+	strcpy(cmd + 2, script);
+	ret = system(cmd);
+	if (ret == -1) {
+		fprintf(stderr, "FATAL: Could not run %s", cmd);
+		fatal_occured++;
+		return 1;
+	}
+	return ret;
+}
+
+
+/* checks for existence of a file and runs it */
+int run_shell(const char *script)
+{
+	struct stat sb;
+
+	if (stat(script, &sb) == -1) {
+		fatal_occured++;
+		return 1;
+	}
+
+	if (! (sb.st_mode & (S_IXUSR | S_IFREG))) {
+		fatal_occured++;
+		return 1;
+	}
+
+	return _run_shell(script);
+}
+
+
+/* if a script is available, run it, otherwise ignore it */
+int try_shell(const char *script)
+{
+	struct stat sb;
+
+	if (stat(script, &sb) == -1)
+		return 0;
+
+	if (! (sb.st_mode & (S_IXUSR | S_IFREG)))
+		return 0;
+
+	return _run_shell(script);
+}
+
+
+/* runs a group of tests ("base_something", ...) */
+int run_group(const char *path)
+{
+	DIR *dp;
+	struct dirent *ep;
+	int ret;
+
+	int failures = 0;
+	ret = chdir(path);
+
+	if (verbose)
+		printf("======== %s ========\n", path);
+
+	/* try to run setup */
+	failures += try_shell("setup.sh");
+
+	/* scan the dir and run tests */
+	dp = opendir("./");
+	if (dp != NULL) {
+		while ((ep = readdir(dp))) {
+			if (strncmp(ep->d_name, "test_", 5))
+				continue;
+			failures += run_shell(ep->d_name);
+		}
+		closedir(dp);
+	}
+	else
+		perror("Cannot open inner dir.");
+
+	/* try to do clean-up */
+	try_shell("cleanup.sh");
+
+	ret = chdir("..");
+	ret = ret;
+	if (verbose)
+		printf("\n");
+	return failures;
+}
+
+
+/* main test */
+int test__suite(int subtest __maybe_unused)
+{
+	DIR *dp;
+	struct dirent *ep;
+	int failures = 0;
+	int ret;
+	int test_status = TEST_OK;
+
+	if (verbose > 0)
+		setenv("TESTMODE_QUIET", "n", 1);
+	else
+		setenv("TESTMODE_QUIET", "y", 1);
+
+	cwd = getcwd(NULL, 0);
+	ret = chdir(TESTSUITE_ROOT);
+	if (ret != 0) {
+		free(cwd);
+		return TEST_SKIP;
+	}
+
+	dp = opendir("./");
+	if (dp != NULL) {
+
+		while ((ep = readdir(dp))) {
+			if (strncmp(ep->d_name, "base_", 5))
+				continue;
+			failures += run_group(ep->d_name);
+		}
+		closedir(dp);
+	}
+	else
+		test_status = TEST_SKIP;
+
+	ret = chdir(cwd);
+	ret = ret;
+	free(cwd);
+
+	if(failures || fatal_occured)
+		test_status = TEST_FAIL;
+
+	return test_status;
+}
diff --git a/tools/perf/tests/suite.h b/tools/perf/tests/suite.h
new file mode 100644
index 0000000..53c768b
--- /dev/null
+++ b/tools/perf/tests/suite.h
@@ -0,0 +1,30 @@
+#ifndef PERF_TEST_SUITE_H
+#define PERF_TEST_SUITE_H
+
+#include <linux/compiler.h>
+#include "tests.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <string.h>
+
+
+/* verbosity */
+extern int verbose;
+
+/* runs a shell script */
+int _run_shell(const char *script);
+
+/* checks for existence of a file and runs it */
+int run_shell(const char *script);
+
+/* if a script is available, run it, otherwise ignore it */
+int try_shell(const char *script);
+
+/* runs a group of tests ("base_something", ...) */
+int run_group(const char *path);
+
+#endif
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index a0733aa..9ad6bd4 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -79,6 +79,7 @@ int test__bpf(int subtest);
 const char *test__bpf_subtest_get_desc(int subtest);
 int test__bpf_subtest_get_nr(void);
 int test_session_topology(int subtest);
+int test__suite(int subtest);
 
 #if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2015-12-07 18:52 [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite Michael Petlan
@ 2015-12-15 12:03 ` Jiri Olsa
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2015-12-15 12:03 UTC (permalink / raw)
  To: Michael Petlan; +Cc: Arnaldo Carvalho de Melo, linux-perf-users

On Mon, Dec 07, 2015 at 07:52:59PM +0100, Michael Petlan wrote:
> In order to be able to run the bash/subcommand-based testsuite directly
> from perf test a new perf-test entry has been created for it.
> 
> The driver (suite.c) runs all the tests  that it finds in the testsuite
> directory: tools/perf/testsuite/   (rather ./testsuite as it expects it
> it being run from the perf's directory).
> 
> In case the testsuite is not found, the testcase is skipped. If this is
> not the desired behaviour, some packaging will be needed.
> 
> Usage:
> 	./perf test suite
> 
> or
> 	./perf test suite -v

so -v has following output now:

	--- SNIP
	[jolsa@krava perf]$ sudo ./perf test suite -v
	38: Testsuite                                                :
	--- start ---
	test child forked, pid 11670
	======== base_annotate ========
	-- [ PASS ] -- perf_annotate :: setup :: building the example code
	Line did not match any pattern: "Lowering default frequency rate to 2000."
	Line did not match any pattern: "Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate."
	-- [ FAIL ] -- perf_annotate :: setup :: record data (output regexp parsing)
	## [ FAIL ] ## perf_annotate :: setup SUMMARY :: 1 failures found
	-- [ SKIP ] -- perf_annotate :: test_basic :: help message :: testcase skipped
	-- [ PASS ] -- perf_annotate :: test_basic :: basic execution - annotate
	-- [ PASS ] -- perf_annotate :: test_basic :: dso filter
	-- [ PASS ] -- perf_annotate :: test_basic :: no-source
	-- [ PASS ] -- perf_annotate :: test_basic :: full-paths
	-- [ PASS ] -- perf_annotate :: test_basic :: print-line
	-- [ PASS ] -- perf_annotate :: test_basic :: redirected input
	-- [ PASS ] -- perf_annotate :: test_basic :: execution without data
	## [ PASS ] ## perf_annotate :: test_basic SUMMARY
	-- [ PASS ] -- perf_annotate :: cleanup :: clean-up logs

	======== base_buildid ========
	## [ PASS ] ## perf_buildid-cache :: setup SUMMARY
	-- [ SKIP ] -- perf_buildid-cache :: test_buildid-list :: help message :: testcase skipped
	Line did not match any pattern: "2e228243285efef5d8b66c729703b4aebdd12c86 [kernel.kallsyms]"
	Line did not match any pattern: "b5105437a54662622e6d461cf3036e79e9261f4f [vdso]"
	Usage: file [-bcEhikLlNnprsvz0] [--apple] [--mime-encoding] [--mime-type]
		    [-e testname] [-F separator] [-f namefile] [-m magicfiles] file ...
	       file -C [-m magicfiles]
	       file [--help]
	Usage: file [-bcEhikLlNnprsvz0] [--apple] [--mime-encoding] [--mime-type]
		    [-e testname] [-F separator] [-f namefile] [-m magicfiles] file ...
	       file -C [-m magicfiles]
	       file [--help]
	-- [ FAIL ] -- perf_buildid-cache :: test_buildid-list :: buildids check (output regexp parsing)
	...
	--- SNIP


could you ensure following output:

	[jolsa@krava perf]$ sudo ./perf test suite 
	38: Testsuite                                                : FAILED!

	[jolsa@krava perf]$ sudo ./perf test suite -v
	38: Testsuite                                                :
	--- start ---
	test child forked, pid 11670
	-- [ PASS ] -- perf_annotate :: setup :: building the example code
	-- [ FAIL ] -- perf_annotate :: setup :: record data (output regexp parsing)
	## [ FAIL ] ## perf_annotate :: setup SUMMARY :: 1 failures found
	-- [ SKIP ] -- perf_annotate :: test_basic :: help message :: testcase skipped
	-- [ PASS ] -- perf_annotate :: test_basic :: basic execution - annotate
	-- [ PASS ] -- perf_annotate :: test_basic :: dso filter
	-- [ PASS ] -- perf_annotate :: test_basic :: no-source
	-- [ PASS ] -- perf_annotate :: test_basic :: full-paths
	-- [ PASS ] -- perf_annotate :: test_basic :: print-line
	-- [ PASS ] -- perf_annotate :: test_basic :: redirected input
	-- [ PASS ] -- perf_annotate :: test_basic :: execution without data
	## [ PASS ] ## perf_annotate :: test_basic SUMMARY
	-- [ PASS ] -- perf_annotate :: cleanup :: clean-up logs
	## [ PASS ] ## perf_buildid-cache :: setup SUMMARY
	-- [ SKIP ] -- perf_buildid-cache :: test_buildid-list :: help message :: testcase skipped
	-- [ FAIL ] -- perf_buildid-cache :: test_buildid-list :: buildids check (output regexp parsing)
	...

	[jolsa@krava perf]$ sudo ./perf test suite -vv
	unleash the hell ;-)

	or even add -vvv ... would be suite developers choice anyway ;-)


thanks,
jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-29 12:19     ` Michael Petlan
@ 2016-03-29 19:39       ` Jiri Olsa
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2016-03-29 19:39 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Tue, Mar 29, 2016 at 02:19:17PM +0200, Michael Petlan wrote:
> On Sun, 27 Mar 2016, Jiri Olsa wrote:
> >On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:
> >
> >SNIP
> >
> >>+	ret = chdir("..");
> >>+	ret = ret;
> >
> >hm? ;-)
> 
> That must be there, otherwise the compiler would complain about unused
> variable 'ret'. When removing 'ret' at all, the compiler complains about
> ignoring return value of 'chdir()'.
> 
> Well, ok, there could be something like:
> 
> if(!ret)
> {
> 		fprintf("FATAL: Cannot cd to ..\n");
> 		return 1;
> }

I dont see a point of having the ret variable at all then..

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-27 12:23   ` Jiri Olsa
@ 2016-03-29 12:57     ` Michael Petlan
  0 siblings, 0 replies; 13+ messages in thread
From: Michael Petlan @ 2016-03-29 12:57 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: linux-perf-users, acme

[-- Attachment #1: Type: text/plain, Size: 2265 bytes --]

On Sun, 27 Mar 2016, Jiri Olsa wrote:
> On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:
>
> SNIP
>
>> +#define TESTSUITE_ROOT "./testsuite/"
>> +
>> +/* globals */
>> +int fatal_occured = 0;
>> +char *cwd;
>> +
>> +
>> +/* runs a shell script */
>> +int _run_shell(const char *script)
>> +{
>> +	int ret;
>> +	char *cmd = malloc(strlen(script) + 3 * sizeof(char));
>
> please check the return value

Fixed in the attached patch.


To the other notes:

> 'cwd' does not need to be global

Fixed in the attached patch.

> you propage all the error all the way through, so what's
> the point of having extra global fatal_occured variable?

It is reserved for distinguishing between a regular failures in the
tests (failures of perf in the ideal state) and the failures of the
infrastructure (cannot allocate memory for the *cmd string above).

     if(failures || fatal_occurred)
         test_status = TEST_FAIL;

I know that this idiom merges that into TEST_FAIL, but we still are
able to make differences here. The suite is pretty complex, I think
there should be a way to notice infrastructure failures.

> also I think you could erase the  ==== quoted lines

I think it is more human-eye-friendly when the sections are separated
by one more \n and the header. I moved that into the (verbose >= 2)
mode. Once there might be twice or thrice more lines and sections as
well. When reviewing the results of the suite, I like the headers.

> is there a point to run tests if the setup fails?

There is a reason, since not all of the tests of the 'group'
(base_something/test_*.sh) need the setup.


Thanks for the notes. The remaining two I have to check separately.
All the failures that I usually encounter have their stderrs cut
out in -v, so the testsuite logs look still nice. I have to check
the failure you posted.

Regarding to the C-c, yes, there are commands running in background,
we need that e.g. for `perf stat -p` testing. However I don't think
that that is why it cannot be C-c'd. I have to look at it more.

Michael

>
> jirka
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[-- Attachment #2: Type: text/plain, Size: 2007 bytes --]

diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
index 17f6610..b3310e8 100644
--- a/tools/perf/tests/suite.c
+++ b/tools/perf/tests/suite.c
@@ -3,8 +3,7 @@
 #define TESTSUITE_ROOT "./testsuite/"
 
 /* globals */
-int fatal_occured = 0;
-char *cwd;
+int fatal_occurred = 0;
 
 
 /* runs a shell script */
@@ -12,12 +11,17 @@ int _run_shell(const char *script)
 {
 	int ret;
 	char *cmd = malloc(strlen(script) + 3 * sizeof(char));
+	if(cmd == NULL)
+	{
+		fprintf(stderr, "FATAL: Could not allocate memory.\n");
+		return 1;
+	}
 	strcpy(cmd, "./");
 	strcpy(cmd + 2, script);
 	ret = system(cmd);
 	if (ret == -1) {
 		fprintf(stderr, "FATAL: Could not run %s", cmd);
-		fatal_occured++;
+		fatal_occurred++;
 		return 1;
 	}
 	return ret;
@@ -30,12 +34,12 @@ int run_shell(const char *script)
 	struct stat sb;
 
 	if (stat(script, &sb) == -1) {
-		fatal_occured++;
+		fatal_occurred++;
 		return 1;
 	}
 
 	if (! (sb.st_mode & (S_IXUSR | S_IFREG))) {
-		fatal_occured++;
+		fatal_occurred++;
 		return 1;
 	}
 
@@ -68,7 +72,7 @@ int run_group(const char *path)
 	int failures = 0;
 	ret = chdir(path);
 
-	if (verbose)
+	if (verbose > 1)
 		printf("======== %s ========\n", path);
 
 	/* try to run setup */
@@ -102,13 +106,14 @@ int run_group(const char *path)
 int test__suite(int subtest __maybe_unused)
 {
 	DIR *dp;
+	char *cwd;
 	struct dirent *ep;
 	int failures = 0;
 	int ret;
 	int test_status = TEST_OK;
 	char verbosity_str[2];
 	int _verbose = (verbose > 9)? 9 : verbose;
-	char tempdir_template[24], _tempdir_template;
+	char tempdir_template[24], *_tempdir_template;
 
 	/* set verbosity according to perf-test command line switches */
 	snprintf(verbosity_str, 2, "%i", _verbose);
@@ -143,7 +148,7 @@ int test__suite(int subtest __maybe_unused)
 	ret = ret;
 	free(cwd);
 
-	if(failures || fatal_occured)
+	if(failures || fatal_occurred)
 		test_status = TEST_FAIL;
 
 	return test_status;

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-27 12:24   ` Jiri Olsa
@ 2016-03-29 12:19     ` Michael Petlan
  0 siblings, 0 replies; 13+ messages in thread
From: Michael Petlan @ 2016-03-29 12:19 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: linux-perf-users, acme

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

On Sun, 27 Mar 2016, Jiri Olsa wrote:
>
> got following compile error:
>
>  CC       tests/suite.o
> tests/suite.c: In function ‘test__suite’:
> tests/suite.c:119:20: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
>  _tempdir_template = mkdtemp(tempdir_template);
>                    ^
> tests/suite.c:120:30: error: passing argument 2 of ‘setenv’ makes pointer from integer without a cast [-Werror=int-conversion]
>  setenv("PERFSUITE_RUN_DIR", _tempdir_template, 1);
>                              ^
> In file included from tests/suite.h:6:0,
>                 from tests/suite.c:1:
> /usr/include/stdlib.h:584:12: note: expected ‘const char *’ but argument is of type ‘char’
> extern int setenv (const char *__name, const char *__value, int __replace)
>            ^
> cc1: all warnings being treated as errors
>
>
> moving on with attached patch

I noticed this error too and that's why I sent the mail with the following $subj:
"[PATCH 1/9] perf test: Adding an entry for the perftool-testsuite - CORRECT"

A stupid mistake, of course it must be a pointer.

Michael

>
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
> index 17f66107f34d..1561bec3a2e1 100644
> --- a/tools/perf/tests/suite.c
> +++ b/tools/perf/tests/suite.c
> @@ -108,7 +108,8 @@ int test__suite(int subtest __maybe_unused)
> 	int test_status = TEST_OK;
> 	char verbosity_str[2];
> 	int _verbose = (verbose > 9)? 9 : verbose;
> -	char tempdir_template[24], _tempdir_template;
> +	char tempdir_template[24];
> +	char *_tempdir_template;
>
> 	/* set verbosity according to perf-test command line switches */
> 	snprintf(verbosity_str, 2, "%i", _verbose);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-27 12:24   ` Jiri Olsa
@ 2016-03-29 12:19     ` Michael Petlan
  2016-03-29 19:39       ` Jiri Olsa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Petlan @ 2016-03-29 12:19 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: linux-perf-users, acme

On Sun, 27 Mar 2016, Jiri Olsa wrote:
> On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:
>
> SNIP
>
>> +	ret = chdir("..");
>> +	ret = ret;
>
> hm? ;-)

That must be there, otherwise the compiler would complain about unused
variable 'ret'. When removing 'ret' at all, the compiler complains about
ignoring return value of 'chdir()'.

Well, ok, there could be something like:

if(!ret)
{
 		fprintf("FATAL: Cannot cd to ..\n");
 		return 1;
}

Do you think it is worth of being there instead?

>
> jirka
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
                     ` (4 preceding siblings ...)
  2016-03-27 12:24   ` Jiri Olsa
@ 2016-03-27 12:24   ` Jiri Olsa
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:24 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:

SNIP

> +/* runs a group of tests ("base_something", ...) */
> +int run_group(const char *path)
> +{
> +	DIR *dp;
> +	struct dirent *ep;
> +	int ret;
> +
> +	int failures = 0;
> +	ret = chdir(path);
> +
> +	if (verbose)
> +		printf("======== %s ========\n", path);
> +
> +	/* try to run setup */
> +	failures += try_shell("setup.sh");

is there a point to run tests if the setup fails?

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
                     ` (3 preceding siblings ...)
  2016-03-27 12:24   ` Jiri Olsa
@ 2016-03-27 12:24   ` Jiri Olsa
  2016-03-29 12:19     ` Michael Petlan
  2016-03-27 12:24   ` Jiri Olsa
  5 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:24 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:

SNIP

> +/* runs a group of tests ("base_something", ...) */
> +int run_group(const char *path)
> +{
> +	DIR *dp;
> +	struct dirent *ep;
> +	int ret;
> +
> +	int failures = 0;
> +	ret = chdir(path);
> +
> +	if (verbose)
> +		printf("======== %s ========\n", path);
> +
> +	/* try to run setup */
> +	failures += try_shell("setup.sh");
> +
> +	/* scan the dir and run tests */
> +	dp = opendir("./");
> +	if (dp != NULL) {
> +		while ((ep = readdir(dp))) {
> +			if (strncmp(ep->d_name, "test_", 5))
> +				continue;
> +			failures += run_shell(ep->d_name);
> +		}
> +		closedir(dp);
> +	}
> +	else
> +		perror("Cannot open inner dir.");
> +
> +	/* try to do clean-up */
> +	try_shell("cleanup.sh");
> +
> +	ret = chdir("..");
> +	ret = ret;

hm? ;-)

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
                     ` (2 preceding siblings ...)
  2016-03-27 12:24   ` Jiri Olsa
@ 2016-03-27 12:24   ` Jiri Olsa
  2016-03-29 12:19     ` Michael Petlan
  2016-03-27 12:24   ` Jiri Olsa
  2016-03-27 12:24   ` Jiri Olsa
  5 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:24 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:
> In order to be able to run the bash/subcommand-based testsuite directly
> from perf test a new perf-test entry has been created for it.
> 
> The driver (suite.c) runs all the tests  that it finds in the testsuite
> directory: tools/perf/testsuite/   (rather ./testsuite as it expects it
> it being run from the perf's directory).
> 
> In case the testsuite is not found, the testcase is skipped. If this is
> not the desired behaviour, some packaging will be needed.
> 
> Usage:
>     ./perf test suite
> 
> or
>     ./perf test suite -v
>     ./perf test suite -vv
> 
> (for the verbose output)
> 
> Signed-off-by: Michael Petlan <mpetlan@redhat.com>

got following compile error:

  CC       tests/suite.o
tests/suite.c: In function ‘test__suite’:
tests/suite.c:119:20: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
  _tempdir_template = mkdtemp(tempdir_template);
                    ^
tests/suite.c:120:30: error: passing argument 2 of ‘setenv’ makes pointer from integer without a cast [-Werror=int-conversion]
  setenv("PERFSUITE_RUN_DIR", _tempdir_template, 1);
                              ^
In file included from tests/suite.h:6:0,
                 from tests/suite.c:1:
/usr/include/stdlib.h:584:12: note: expected ‘const char *’ but argument is of type ‘char’
 extern int setenv (const char *__name, const char *__value, int __replace)
            ^
cc1: all warnings being treated as errors


moving on with attached patch

thanks,
jirka


---
diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
index 17f66107f34d..1561bec3a2e1 100644
--- a/tools/perf/tests/suite.c
+++ b/tools/perf/tests/suite.c
@@ -108,7 +108,8 @@ int test__suite(int subtest __maybe_unused)
 	int test_status = TEST_OK;
 	char verbosity_str[2];
 	int _verbose = (verbose > 9)? 9 : verbose;
-	char tempdir_template[24], _tempdir_template;
+	char tempdir_template[24];
+	char *_tempdir_template;
 
 	/* set verbosity according to perf-test command line switches */
 	snprintf(verbosity_str, 2, "%i", _verbose);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
  2016-03-27 12:23   ` Jiri Olsa
  2016-03-27 12:23   ` Jiri Olsa
@ 2016-03-27 12:24   ` Jiri Olsa
  2016-03-27 12:24   ` Jiri Olsa
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:24 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:

SNIP

> +	dp = opendir("./");
> +	if (dp != NULL) {
> +
> +		while ((ep = readdir(dp))) {
> +			if (strncmp(ep->d_name, "base_", 5))
> +				continue;
> +			failures += run_group(ep->d_name);
> +		}
> +		closedir(dp);
> +	}
> +	else
> +		test_status = TEST_SKIP;
> +
> +	ret = chdir(cwd);
> +	ret = ret;
> +	free(cwd);
> +
> +	if(failures || fatal_occured)

you propage all the error all the way through, so what's
the point of having extra global fatal_occured variable?

thanks,
jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
  2016-03-27 12:23   ` Jiri Olsa
@ 2016-03-27 12:23   ` Jiri Olsa
  2016-03-27 12:24   ` Jiri Olsa
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:23 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:

SNIP

> diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
> new file mode 100644
> index 0000000..17f6610
> --- /dev/null
> +++ b/tools/perf/tests/suite.c
> @@ -0,0 +1,150 @@
> +#include "suite.h"
> +
> +#define TESTSUITE_ROOT "./testsuite/"
> +
> +/* globals */
> +int fatal_occured = 0;
> +char *cwd;

'cwd' does not need to be global

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
  2016-03-16 13:50 ` Michael Petlan
@ 2016-03-27 12:23   ` Jiri Olsa
  2016-03-29 12:57     ` Michael Petlan
  2016-03-27 12:23   ` Jiri Olsa
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2016-03-27 12:23 UTC (permalink / raw)
  To: Michael Petlan; +Cc: linux-perf-users, acme

On Wed, Mar 16, 2016 at 02:50:59PM +0100, Michael Petlan wrote:

SNIP

> +#define TESTSUITE_ROOT "./testsuite/"
> +
> +/* globals */
> +int fatal_occured = 0;
> +char *cwd;
> +
> +
> +/* runs a shell script */
> +int _run_shell(const char *script)
> +{
> +	int ret;
> +	char *cmd = malloc(strlen(script) + 3 * sizeof(char));

please check the return value

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite
       [not found] <cover.1458134357.git.mpetlan@redhat.com>
@ 2016-03-16 13:50 ` Michael Petlan
  2016-03-27 12:23   ` Jiri Olsa
                     ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Michael Petlan @ 2016-03-16 13:50 UTC (permalink / raw)
  To: linux-perf-users; +Cc: acme, Jiri Olsa

In order to be able to run the bash/subcommand-based testsuite directly
from perf test a new perf-test entry has been created for it.

The driver (suite.c) runs all the tests  that it finds in the testsuite
directory: tools/perf/testsuite/   (rather ./testsuite as it expects it
it being run from the perf's directory).

In case the testsuite is not found, the testcase is skipped. If this is
not the desired behaviour, some packaging will be needed.

Usage:
     ./perf test suite

or
     ./perf test suite -v
     ./perf test suite -vv

(for the verbose output)

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
---
  tools/perf/tests/Build          |   1 +
  tools/perf/tests/builtin-test.c |   4 ++
  tools/perf/tests/suite.c        | 150 ++++++++++++++++++++++++++++++++++++++++
  tools/perf/tests/suite.h        |  30 ++++++++
  tools/perf/tests/tests.h        |   1 +
  5 files changed, 186 insertions(+)
  create mode 100644 tools/perf/tests/suite.c
  create mode 100644 tools/perf/tests/suite.h

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 1ba628e..c26fd8b 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -37,6 +37,7 @@ perf-y += topology.o
  perf-y += cpumap.o
  perf-y += stat.o
  perf-y += event_update.o
+perf-y += suite.o

  $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
  	$(call rule_mkdir)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index f2b1dca..14246a1 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -204,6 +204,10 @@ static struct test generic_tests[] = {
  		.func = test__event_update,
  	},
  	{
+		.desc = "Testsuite",
+		.func = test__suite,
+	},
+	{
  		.func = NULL,
  	},
  };
diff --git a/tools/perf/tests/suite.c b/tools/perf/tests/suite.c
new file mode 100644
index 0000000..17f6610
--- /dev/null
+++ b/tools/perf/tests/suite.c
@@ -0,0 +1,150 @@
+#include "suite.h"
+
+#define TESTSUITE_ROOT "./testsuite/"
+
+/* globals */
+int fatal_occured = 0;
+char *cwd;
+
+
+/* runs a shell script */
+int _run_shell(const char *script)
+{
+	int ret;
+	char *cmd = malloc(strlen(script) + 3 * sizeof(char));
+	strcpy(cmd, "./");
+	strcpy(cmd + 2, script);
+	ret = system(cmd);
+	if (ret == -1) {
+		fprintf(stderr, "FATAL: Could not run %s", cmd);
+		fatal_occured++;
+		return 1;
+	}
+	return ret;
+}
+
+
+/* checks for existence of a file and runs it */
+int run_shell(const char *script)
+{
+	struct stat sb;
+
+	if (stat(script, &sb) == -1) {
+		fatal_occured++;
+		return 1;
+	}
+
+	if (! (sb.st_mode & (S_IXUSR | S_IFREG))) {
+		fatal_occured++;
+		return 1;
+	}
+
+	return _run_shell(script);
+}
+
+
+/* if a script is available, run it, otherwise ignore it */
+int try_shell(const char *script)
+{
+	struct stat sb;
+
+	if (stat(script, &sb) == -1)
+		return 0;
+
+	if (! (sb.st_mode & (S_IXUSR | S_IFREG)))
+		return 0;
+
+	return _run_shell(script);
+}
+
+
+/* runs a group of tests ("base_something", ...) */
+int run_group(const char *path)
+{
+	DIR *dp;
+	struct dirent *ep;
+	int ret;
+
+	int failures = 0;
+	ret = chdir(path);
+
+	if (verbose)
+		printf("======== %s ========\n", path);
+
+	/* try to run setup */
+	failures += try_shell("setup.sh");
+
+	/* scan the dir and run tests */
+	dp = opendir("./");
+	if (dp != NULL) {
+		while ((ep = readdir(dp))) {
+			if (strncmp(ep->d_name, "test_", 5))
+				continue;
+			failures += run_shell(ep->d_name);
+		}
+		closedir(dp);
+	}
+	else
+		perror("Cannot open inner dir.");
+
+	/* try to do clean-up */
+	try_shell("cleanup.sh");
+
+	ret = chdir("..");
+	ret = ret;
+	if (verbose)
+		printf("\n");
+	return failures;
+}
+
+
+/* main test */
+int test__suite(int subtest __maybe_unused)
+{
+	DIR *dp;
+	struct dirent *ep;
+	int failures = 0;
+	int ret;
+	int test_status = TEST_OK;
+	char verbosity_str[2];
+	int _verbose = (verbose > 9)? 9 : verbose;
+	char tempdir_template[24], _tempdir_template;
+
+	/* set verbosity according to perf-test command line switches */
+	snprintf(verbosity_str, 2, "%i", _verbose);
+	setenv("TESTLOG_VERBOSITY", verbosity_str, 1);
+
+	/* always place all temp files and logs elsewhere */
+	strcpy(tempdir_template, "/tmp/perfsuite-XXXXXX");
+	_tempdir_template = mkdtemp(tempdir_template);
+	setenv("PERFSUITE_RUN_DIR", _tempdir_template, 1);
+
+	cwd = getcwd(NULL, 0);
+	ret = chdir(TESTSUITE_ROOT);
+	if (ret != 0) {
+		free(cwd);
+		return TEST_SKIP;
+	}
+
+	dp = opendir("./");
+	if (dp != NULL) {
+
+		while ((ep = readdir(dp))) {
+			if (strncmp(ep->d_name, "base_", 5))
+				continue;
+			failures += run_group(ep->d_name);
+		}
+		closedir(dp);
+	}
+	else
+		test_status = TEST_SKIP;
+
+	ret = chdir(cwd);
+	ret = ret;
+	free(cwd);
+
+	if(failures || fatal_occured)
+		test_status = TEST_FAIL;
+
+	return test_status;
+}
diff --git a/tools/perf/tests/suite.h b/tools/perf/tests/suite.h
new file mode 100644
index 0000000..53c768b
--- /dev/null
+++ b/tools/perf/tests/suite.h
@@ -0,0 +1,30 @@
+#ifndef PERF_TEST_SUITE_H
+#define PERF_TEST_SUITE_H
+
+#include <linux/compiler.h>
+#include "tests.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <string.h>
+
+
+/* verbosity */
+extern int verbose;
+
+/* runs a shell script */
+int _run_shell(const char *script);
+
+/* checks for existence of a file and runs it */
+int run_shell(const char *script);
+
+/* if a script is available, run it, otherwise ignore it */
+int try_shell(const char *script);
+
+/* runs a group of tests ("base_something", ...) */
+int run_group(const char *path);
+
+#endif
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 82b2b5e..af516de 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -85,6 +85,7 @@ int test__synthesize_stat_config(int subtest);
  int test__synthesize_stat(int subtest);
  int test__synthesize_stat_round(int subtest);
  int test__event_update(int subtest);
+int test__suite(int subtest);

  #if defined(__arm__) || defined(__aarch64__)
  #ifdef HAVE_DWARF_UNWIND_SUPPORT
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-03-29 19:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-07 18:52 [PATCH 1/9] perf test: Adding an entry for the perftool-testsuite Michael Petlan
2015-12-15 12:03 ` Jiri Olsa
     [not found] <cover.1458134357.git.mpetlan@redhat.com>
2016-03-16 13:50 ` Michael Petlan
2016-03-27 12:23   ` Jiri Olsa
2016-03-29 12:57     ` Michael Petlan
2016-03-27 12:23   ` Jiri Olsa
2016-03-27 12:24   ` Jiri Olsa
2016-03-27 12:24   ` Jiri Olsa
2016-03-29 12:19     ` Michael Petlan
2016-03-27 12:24   ` Jiri Olsa
2016-03-29 12:19     ` Michael Petlan
2016-03-29 19:39       ` Jiri Olsa
2016-03-27 12:24   ` Jiri Olsa

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.