Re: [Piglit] [PATCH 3/4] util: Add a -list-subtests option that will list all the subtests
On Fri, Oct 04, 2013 at 06:11:00PM -0700, Ian Romanick wrote:
From: Ian Romanick ian.d.roman...@intel.com
This required some ugly hacking about to get the list of subtests to
process_args. Suggestions?
The utility is that all.tests can do 'sometest -list-subtests' to
automatically get the list of subtests to run. Since the syntax for
selecting subtests is regularized, this works quite well.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Cc: Chad Versace chad.vers...@linux.intel.com
Since intel-gpu-tools has a very similar --list-subtest and --run-subtest
I guess it can't hurt if I quickly explain what we have. Most parts are
very similar (e.g. the arg parsing stuff to setup the test suite), the big
difference is that we don't expect tests to supply a static list of
subtests. Instead there's rather magic
bool __igt_run_subtest(const char *subtest_name);
which returns true if the subtest should be run, false otherwise. In
normal mode that's all it does, but if the --list-subtest parameter has
been detected in the arg parsing function it will always return false but
instead print just print out all the testcase names. This is then wrapped
into some magic macros to create subtest blocks:
igt_subtest(foo) {
/* code for subtest foo */
}
The clear downside is that the global code in tests needs to be written
more carefully (to avoid polluting the subtest list, among other things).
To help that we have more magic code blocks to mark such global setup
sections and prevent them from causing havoc when listing subtest:
igt_fixture {
/* global setup code goes here, not run when enumerating subtests */
}
The really great upside is that it's trivially easy to construct subtests
programmatically at runtime. This is great if you have combinatorial tests
with twists, e.g.
for (fmt in formats) {
for (flag = 0; flag FLAG_MASK + 1; flag++) {
igt_subtest_f(normal-%s%s%s%s, fmt_to_str(fmt),
flag FLAG_FOO ? -foo : ,
flag FLAG_BAR ? -bar : ,
flag CRAZY ? -crazy : )
run_tests(fmt, flag);
if (surface_sane(fmt) !(flag CRAZY)) {
igt_subtest_f(normal-%s%s%s%s, fmt_to_str(fmt),
flag FLAG_FOO ? -foo : ,
flag FLAG_BAR ? -bar : ,
flag CRAZY ? -crazy : ) {
setup_different_env();
run_tests(fmt, flag);
teardown_different_env();
}
}
}
}
In igt we have ridiculous piles of tests structured like this where some
testscase with special flags can't be run in some special enviroments. So
the static list was something I've considered but then ditched again.
Anyway, just my 2 cents.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 3/4] util: Add a -list-subtests option that will list all the subtests
On 10/04/2013 06:11 PM, Ian Romanick wrote:
From: Ian Romanick ian.d.roman...@intel.com
This required some ugly hacking about to get the list of subtests to
process_args. Suggestions?
Let's avoid these ugly hacks. Here's my suggestion, which covers patches 2 and
3.
Patch 2/4 moves `struct piglit_gl_test_config config` out of main() to
file scope. Let's avoid making things global; that leads to a slippery
slope. I fought hard to kill many of Piglit's globals, and I don't welcome
the arrival of new ones. If at all possible, let's leave it in main() where it
is.
To leave 'config' in main(), we need to restructure a few things. First,
piglit_gl_test_config_init() should no longer parse args; instead, it should
only
memset 'config' to 0. That is, let's change its signature from
void piglit_gl_test_config_init(int *argc, char *argv[], struct
piglit_gl_test_config *config)
to
void piglit_gl_test_config_init(struct piglit_gl_test_config *config)
Move the argparsing (that is, the call to process_args()) into
piglit_gl_test_run(). That will ensure
that args are parsed *after* the test author has set all config variables in
the CONFIG block.
The CONFIG block in your patch 4/4 should now look like this:
PIGLIT_GL_TEST_CONFIG_BEGIN
config.subtests = subtests;
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
PIGLIT_GL_TEST_CONFIG_END
The config is no longer defined with file-scope, so piglit_init() should
execute subtests
like this:
result = piglit_run_subtests(PIGLIT_SKIP);
piglit_run_selected_subtests() should access `config-subtests` through the
global
test context object, piglit-framework-gl.c:`struct piglit_gl_framework
*gl_fw`,
like this:
enum piglit_result
piglit_run_selected_subtests(enum piglit_result previous_result)
{
enum piglit_result result = previous_result;
const struct piglit_gl_subtest *subtests
gl_fw-config-subtests;
const char **selected_subtests =
gl_fw-config-selected_subtests;
...
}
At this point, it's no longer necessary for the subtests array to be global,
so you could even do the below if you wanted, but that's really up to personal
taste.
PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
config.subtests = {
{
Cannot delete while active,
delete-inactive-only,
delete_inactive_only
},
...,
};
PIGLIT_GL_TEST_CONFIG_END
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 3/4] util: Add a -list-subtests option that will list all the subtests
On 10/07/2013 11:02 AM, Chad Versace wrote:
On 10/04/2013 06:11 PM, Ian Romanick wrote:
From: Ian Romanick ian.d.roman...@intel.com
This required some ugly hacking about to get the list of subtests to
process_args. Suggestions?
Let's avoid these ugly hacks. Here's my suggestion, which covers patches
2 and 3.
My troll bait worked. :)
Patch 2/4 moves `struct piglit_gl_test_config config` out of main() to
file scope. Let's avoid making things global; that leads to a slippery
slope. I fought hard to kill many of Piglit's globals, and I don't welcome
the arrival of new ones. If at all possible, let's leave it in main()
where it is.
To leave 'config' in main(), we need to restructure a few things. First,
piglit_gl_test_config_init() should no longer parse args; instead, it
should only
memset 'config' to 0. That is, let's change its signature from
void piglit_gl_test_config_init(int *argc, char *argv[], struct
piglit_gl_test_config *config)
to
void piglit_gl_test_config_init(struct piglit_gl_test_config *config)
Move the argparsing (that is, the call to process_args()) into
piglit_gl_test_run(). That will ensure
that args are parsed *after* the test author has set all config
variables in the CONFIG block.
The CONFIG block in your patch 4/4 should now look like this:
PIGLIT_GL_TEST_CONFIG_BEGIN
config.subtests = subtests;
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
PIGLIT_GL_TEST_CONFIG_END
I like this. This is the way I originally wanted to structure things,
but there were issues.
The config is no longer defined with file-scope, so piglit_init() should
execute subtests
like this:
result = piglit_run_subtests(PIGLIT_SKIP);
piglit_run_selected_subtests() should access `config-subtests` through
the global
test context object, piglit-framework-gl.c:`struct piglit_gl_framework
*gl_fw`,
like this:
enum piglit_result
piglit_run_selected_subtests(enum piglit_result previous_result)
{
enum piglit_result result = previous_result;
const struct piglit_gl_subtest *subtests gl_fw-config-subtests;
const char **selected_subtests = gl_fw-config-selected_subtests;
...
}
I have somewhat mixed feelings about this... I liked that the old
piglit_run_selected_subtests was both more primitive (flexible) and more
explicit. This is the birth of this tool, and I'm not sure how the next
user will want to use it. Hmm...
A third option is an accessor that lets the test get config or maybe
just the list of selected tests from the implicit test context object.
At this point, it's no longer necessary for the subtests array to be
global,
so you could even do the below if you wanted, but that's really up to
personal
taste.
PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
config.subtests = {
{
Cannot delete while active,
delete-inactive-only,
delete_inactive_only
},
...,
};
PIGLIT_GL_TEST_CONFIG_END
I think that's C99 syntax, so we probably can't use it in piglit.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 3/4] util: Add a -list-subtests option that will list all the subtests
On 10/07/2013 04:19 PM, Ian Romanick wrote:
On 10/07/2013 11:02 AM, Chad Versace wrote:
On 10/04/2013 06:11 PM, Ian Romanick wrote:
From: Ian Romanick ian.d.roman...@intel.com
This required some ugly hacking about to get the list of subtests to
process_args. Suggestions?
Let's avoid these ugly hacks. Here's my suggestion, which covers patches
2 and 3.
My troll bait worked. :)
Damn... you caught me...
Patch 2/4 moves `struct piglit_gl_test_config config` out of main() to
file scope. Let's avoid making things global; that leads to a slippery
slope. I fought hard to kill many of Piglit's globals, and I don't welcome
the arrival of new ones. If at all possible, let's leave it in main()
where it is.
To leave 'config' in main(), we need to restructure a few things. First,
piglit_gl_test_config_init() should no longer parse args; instead, it
should only
memset 'config' to 0. That is, let's change its signature from
void piglit_gl_test_config_init(int *argc, char *argv[], struct
piglit_gl_test_config *config)
to
void piglit_gl_test_config_init(struct piglit_gl_test_config *config)
Move the argparsing (that is, the call to process_args()) into
piglit_gl_test_run(). That will ensure
that args are parsed *after* the test author has set all config
variables in the CONFIG block.
The CONFIG block in your patch 4/4 should now look like this:
PIGLIT_GL_TEST_CONFIG_BEGIN
config.subtests = subtests;
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
PIGLIT_GL_TEST_CONFIG_END
I like this. This is the way I originally wanted to structure things,
but there were issues.
The config is no longer defined with file-scope, so piglit_init() should
execute subtests
like this:
result = piglit_run_subtests(PIGLIT_SKIP);
piglit_run_selected_subtests() should access `config-subtests` through
the global
test context object, piglit-framework-gl.c:`struct piglit_gl_framework
*gl_fw`,
like this:
enum piglit_result
piglit_run_selected_subtests(enum piglit_result previous_result)
{
enum piglit_result result = previous_result;
const struct piglit_gl_subtest *subtests gl_fw-config-subtests;
const char **selected_subtests = gl_fw-config-selected_subtests;
...
}
I have somewhat mixed feelings about this... I liked that the old
piglit_run_selected_subtests was both more primitive (flexible) and more
explicit. This is the birth of this tool, and I'm not sure how the next
user will want to use it. Hmm...
A third option is an accessor that lets the test get config or maybe
just the list of selected tests from the implicit test context object.
Introducing an accessor for the config makes sense to me.
At this point, it's no longer necessary for the subtests array to be
global,
so you could even do the below if you wanted, but that's really up to
personal
taste.
PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_RGB;
config.subtests = {
{
Cannot delete while active,
delete-inactive-only,
delete_inactive_only
},
...,
};
PIGLIT_GL_TEST_CONFIG_END
I think that's C99 syntax, so we probably can't use it in piglit.
Argh, that's not valid syntax anywhere. You can only initialize an array
like above at the point of variable declaration. However, such initialization
is legal in C89.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 3/4] util: Add a -list-subtests option that will list all the subtests
From: Ian Romanick ian.d.roman...@intel.com
This required some ugly hacking about to get the list of subtests to
process_args. Suggestions?
The utility is that all.tests can do 'sometest -list-subtests' to
automatically get the list of subtests to run. Since the syntax for
selecting subtests is regularized, this works quite well.
Signed-off-by: Ian Romanick ian.d.roman...@intel.com
Cc: Chad Versace chad.vers...@linux.intel.com
---
tests/util/piglit-framework-gl.c | 14 +-
tests/util/piglit-framework-gl.h | 18 --
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/tests/util/piglit-framework-gl.c b/tests/util/piglit-framework-gl.c
index 975c1a9..ea109da 100644
--- a/tests/util/piglit-framework-gl.c
+++ b/tests/util/piglit-framework-gl.c
@@ -45,12 +45,14 @@ process_args(int *argc, char *argv[], unsigned
*force_samples,
void
piglit_gl_test_config_init(int *argc, char *argv[],
- struct piglit_gl_test_config *config)
+ struct piglit_gl_test_config *config,
+ const struct piglit_gl_subtest *subtests)
{
unsigned force_samples = 0;
memset(config, 0, sizeof(*config));
+ config-subtests = subtests;
process_args(argc, argv, force_samples, config);
if (force_samples 1)
@@ -149,6 +151,16 @@ process_args(int *argc, char *argv[], unsigned
*force_samples,
}
*argc -= 2;
j -= 2;
+ } else if (!strcmp(argv[j], -list-subtests)) {
+ unsigned i;
+
+ for (i = 0; config-subtests[i].name != NULL; i++) {
+ printf(%s: %s\n,
+ config-subtests[i].option,
+ config-subtests[i].name);
+ }
+
+ exit(0);
}
}
}
diff --git a/tests/util/piglit-framework-gl.h b/tests/util/piglit-framework-gl.h
index 3357793..d008809 100644
--- a/tests/util/piglit-framework-gl.h
+++ b/tests/util/piglit-framework-gl.h
@@ -184,6 +184,14 @@ struct piglit_gl_test_config {
(*display)(void);
/**
+* List of subtests supported by this test case
+*
+* This is only used during command line argument parsing to implement
+* the -list-subtests option.
+*/
+ const struct piglit_gl_subtest *subtests;
+
+ /**
* Names of subtests supplied on the command line.
*
* The paramaters passed to each -subtest command line option is
@@ -199,7 +207,8 @@ struct piglit_gl_test_config {
*/
void
piglit_gl_test_config_init(int *argc, char *argv[],
- struct piglit_gl_test_config *config);
+ struct piglit_gl_test_config *config,
+ const struct piglit_gl_subtest *subtests);
/**
* Run the OpenGL test described by @a config. Does not return.
@@ -216,6 +225,10 @@ piglit_gl_test_run(int argc, char *argv[],
# define PIGLIT_EXTERN_C_END
#endif
+#ifndef PIGLIT_SUBTEST_LIST
+#define PIGLIT_SUBTEST_LIST NULL
+#endif
+
#define PIGLIT_GL_TEST_CONFIG_BEGIN \
\
PIGLIT_EXTERN_C_BEGIN\
@@ -233,7 +246,8 @@ piglit_gl_test_run(int argc, char *argv[],
int \
main(int argc, char *argv[]) \
{\
-piglit_gl_test_config_init(argc, argv, config);\
+piglit_gl_test_config_init(argc, argv, config, \
+ PIGLIT_SUBTEST_LIST); \
\
config.init = piglit_init; \
config.display = piglit_display; \
--
1.8.1.4
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
