[Piglit] [PATCH 2/4] util: Add common framework for command-line invoked subtests
Ian Romanick
idr at freedesktop.org
Tue Oct 8 09:09:48 PDT 2013
On 10/07/2013 10:39 AM, Chad Versace wrote:
> On 10/04/2013 06:10 PM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> There are two parts to this.
>>
>> First, command line options of the form "-subtest foo" have their
>> argument (the "foo" part) stored in a list in the
>> piglit_gl_test_config. Tests can use this functionality without having
>> to use any of the other parts. This allows every piglit test to have
>> the same syntax for specifying which subtests to run.
>>
>> Second, tests can create a list of piglit_gl_subtest structures. Each
>> structure describes a single subtest: the name (as it will apear in the
>> log), the command-line name (as supplied to -subtest), and the function
>> that implements the test. Helper function use this table and the list
>> of tests specified by -subtest options to run a group of tests.
>>
>> The next patch shows an example of using this functionality.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>> tests/util/piglit-framework-gl.c | 96
>> ++++++++++++++++++++++++++++++++++++++--
>> tests/util/piglit-framework-gl.h | 36 ++++++++++++++-
>> 2 files changed, 127 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/util/piglit-framework-gl.c
>> b/tests/util/piglit-framework-gl.c
>> index 2a315be..975c1a9 100644
>> --- a/tests/util/piglit-framework-gl.c
>> +++ b/tests/util/piglit-framework-gl.c
>> @@ -40,7 +40,8 @@ int piglit_width;
>> int piglit_height;
>>
>> static void
>> -process_args(int *argc, char *argv[], unsigned *force_samples);
>> +process_args(int *argc, char *argv[], unsigned *force_samples,
>> + struct piglit_gl_test_config *config);
>>
>> void
>> piglit_gl_test_config_init(int *argc, char *argv[],
>> @@ -50,7 +51,7 @@ piglit_gl_test_config_init(int *argc, char *argv[],
>>
>> memset(config, 0, sizeof(*config));
>>
>> - process_args(argc, argv, &force_samples);
>> + process_args(argc, argv, &force_samples, config);
>>
>> if (force_samples > 1)
>> config->window_samples = force_samples;
>> @@ -72,7 +73,8 @@ delete_arg(char *argv[], int argc, int arg)
>> * length is returned in @a argc.
>> */
>> static void
>> -process_args(int *argc, char *argv[], unsigned *force_samples)
>> +process_args(int *argc, char *argv[], unsigned *force_samples,
>> + struct piglit_gl_test_config *config)
>> {
>> int j;
>>
>> @@ -120,6 +122,33 @@ process_args(int *argc, char *argv[], unsigned
>> *force_samples)
>> *force_samples = atoi(argv[j]+9);
>> delete_arg(argv, *argc, j--);
>> *argc -= 1;
>> + } else if (!strcmp(argv[j], "-subtest")) {
>> + int i;
>> +
>> + j++;
>> + if (j >= *argc) {
>> + fprintf(stderr,
>> + "-subtest requires an argument\n");
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + config->selected_subtests =
>> + realloc(config->selected_subtests,
>> + sizeof(char *)
>> + * (config->num_selected_subtests + 1));
>> + config->selected_subtests[config->num_selected_subtests] =
>> + argv[j];
>> +
>> + config->num_selected_subtests++;
>> +
>> + /* Remove 2 arguments (hence the 'i - 2') from the
>> + * command line.
>> + */
>> + for (i = j + 1; i < *argc; i++) {
>> + argv[i - 2] = argv[i];
>> + }
>> + *argc -= 2;
>> + j -= 2;
>> }
>> }
>> }
>> @@ -219,3 +248,64 @@ piglit_destroy_dma_buf(struct piglit_dma_buf *buf)
>> if (gl_fw->destroy_dma_buf)
>> gl_fw->destroy_dma_buf(buf);
>> }
>> +
>> +const struct piglit_gl_subtest *
>> +piglit_find_subtest(const struct piglit_gl_subtest *subtests, const
>> char *name)
>> +{
>> + unsigned i;
>> +
>> + for (i = 0; subtests[i].subtest != NULL; i++) {
>> + if (strcmp(subtests[i].option, name) == 0)
>> + return &subtests[i];
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +enum piglit_result
>> +piglit_run_selected_subtests(const struct piglit_gl_subtest
>> *all_subtests,
>> + const char **selected_subtests,
>> + size_t num_selected_subtests,
>> + enum piglit_result previous_result)
>> +{
>> + enum piglit_result result = previous_result;
>> +
>> + if (num_selected_subtests) {
>> + unsigned i;
>> +
>> + for (i = 0; i < num_selected_subtests; i++) {
>> + enum piglit_result subtest_result;
>> + const char *const name = selected_subtests[i];
>> + const struct piglit_gl_subtest *subtest =
>> + piglit_find_subtest(all_subtests, name);
>> +
>> + if (subtest == NULL) {
>> + fprintf(stderr,
>> + "Unknown subtest \"%s\".\n",
>> + name);
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> +
>> + subtest_result = subtest->subtest();
>> + piglit_report_subtest_result(subtest_result,
>> + subtest->name);
>> +
>> + result = piglit_update_result_from_subtest_result(
>> + result, subtest_result);
>> + }
>> + } else {
>> + unsigned i;
>> +
>> + for (i = 0; all_subtests[i].subtest != NULL; i++) {
>> + const enum piglit_result subtest_result =
>> + all_subtests[i].subtest();
>> + piglit_report_subtest_result(subtest_result,
>> + all_subtests[i].name);
>> +
>> + result = piglit_update_result_from_subtest_result(
>> + result, subtest_result);
>> + }
>> + }
>> +
>> + return result;
>> +}
>> diff --git a/tests/util/piglit-framework-gl.h
>> b/tests/util/piglit-framework-gl.h
>> index 7520f38..3357793 100644
>> --- a/tests/util/piglit-framework-gl.h
>> +++ b/tests/util/piglit-framework-gl.h
>> @@ -43,6 +43,20 @@ enum piglit_gl_visual {
>> };
>>
>> /**
>> + * An idividual subtest that makes up part of a test group.
>> + */
>> +struct piglit_gl_subtest {
>> + /**< Name of the subtest as it will appear in the log. */
>> + const char *name;
>> +
>> + /**< Command line name used to select this test. */
>> + const char *option;
>> +
>> + /**< Function that implements the test. */
>> + enum piglit_result (*subtest)(void);
>
> I find it confusing that a struct named 'subtest' has a field named
> 'subtest'.
> Maybe 'func'? Or 'subtest_func'? Or make it a verb, 'run_subtest'? Maybe
> this
> is bikeshedding, but the name did confuse me when I encountered it above in
> piglit_run_selected_subtests().
Yeah, I like subtest_func.
> Also, the Doxygen token '<' should be removed. That token should be used
> only when the comment follows the commented-symbol on the same line,
> like this:
Right. I started off with them end-of-line, but didn't remove the <
when I moved the comments.
> const char *option; /**< Command line name used blah blah blah. /
>
>> +
>> +/**
>> * @brief Configuration for running an OpenGL test.
>> *
>> * To run a test, pass this to piglit_gl_test_run().
>> @@ -168,6 +182,15 @@ struct piglit_gl_test_config {
>> */
>> enum piglit_result
>> (*display)(void);
>> +
>> + /**
>> + * Names of subtests supplied on the command line.
>> + *
>> + * The paramaters passed to each -subtest command line option is
>> + * stored here in the order of appearance on the command line.
>> + */
>> + const char **selected_subtests;
>> + size_t num_selected_subtests;
>> };
>>
>> /**
>> @@ -205,11 +228,11 @@ piglit_gl_test_run(int argc, char *argv[],
>>
>> \
>>
>> PIGLIT_EXTERN_C_END \
>>
>> \
>> + static struct piglit_gl_test_config
>> config; \
>> +
>> \
>>
>> int \
>> main(int argc, char
>> *argv[]) \
>>
>> { \
>> - struct piglit_gl_test_config
>> config; \
>> -
>> \
>> piglit_gl_test_config_init(&argc, argv,
>> &config); \
>>
>> \
>> config.init =
>> piglit_init; \
>> @@ -283,4 +306,13 @@ piglit_create_dma_buf(unsigned w, unsigned h,
>> unsigned cpp,
>> void
>> piglit_destroy_dma_buf(struct piglit_dma_buf *buf);
>>
>> +const struct piglit_gl_subtest *
>> +piglit_find_subtest(const struct piglit_gl_subtest *subtests, const
>> char *name);
>> +
>> +enum piglit_result
>> +piglit_run_selected_subtests(const struct piglit_gl_subtest
>> *all_subtests,
>> + const char **selected_subtests,
>> + size_t num_selected_subtests,
>> + enum piglit_result previous_result);
>> +
>> #endif /* PIGLIT_FRAMEWORK_H */
>>
More information about the Piglit
mailing list