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
