[igt-dev] [PATCH i-g-t 1/4] runner: Introduce --prune_mode to igt_runner
Petri Latvala
petri.latvala at intel.com
Thu Mar 31 11:42:32 UTC 2022
On Thu, Mar 31, 2022 at 03:22:03PM +0530, Sharma, Swati2 wrote:
> The patch series looks good to me.
>
> Reviewed-by:
> Swati Sharma <swati2.sharma at intel.com>
Thanks for the review! Merged.
--
Petri Latvala
>
> On 23-Mar-22 8:50 PM, Petri Latvala wrote:
> > Allow finer control of reporting dynamic subtests instead of
> > unconditionally assuming that the main subtest result and logs are
> > uninteresting if the subtest has dynamic subtests.
> >
> > The default is still to remove subtest results when the subtest has
> > dynamic subtests. Other options are:
> >
> > keep-subtests: Remove the dynamic subtests instead, for cases when a
> > stable test count is more important.
> >
> > keep-all: Remove nothing.
> >
> > keep-requested: Remove the results that were not directly requested to
> > be executed. This option is useful in cases where the test selection
> > is a hand-picked mix of subtests and particular dynamic subtests.
> >
> > Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> > Cc: Arkadiusz Hiler <arek at hiler.eu>
> > ---
> > runner/resultgen.c | 83 +++++++++++++++++++++++++++++++++++-----------
> > runner/settings.c | 49 +++++++++++++++++++++++++++
> > runner/settings.h | 8 +++++
> > 3 files changed, 121 insertions(+), 19 deletions(-)
> >
> > diff --git a/runner/resultgen.c b/runner/resultgen.c
> > index bccfca12..76f9fb7d 100644
> > --- a/runner/resultgen.c
> > +++ b/runner/resultgen.c
> > @@ -1229,19 +1229,64 @@ static void fill_from_journal(int fd,
> > fclose(f);
> > }
> > -static void prune_subtests_with_dynamic_subtests(const char *binary,
> > - struct subtest_list *subtests,
> > - struct json_object *tests)
> > +static bool result_is_requested(struct job_list_entry *entry,
> > + const char *subtestname,
> > + const char *dynamic_name)
> > {
> > - char piglit_name[256];
> > + char entryname[512];
> > size_t i;
> > + if (dynamic_name)
> > + snprintf(entryname, sizeof(entryname) - 1, "%s@%s", subtestname, dynamic_name);
> > + else
> > + strncpy(entryname, subtestname, sizeof(entryname) - 1);
> > +
> > + for (i = 0; i < entry->subtest_count; i++) {
> > + if (!strcmp(entry->subtests[i], entryname))
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static void prune_subtests(struct settings *settings,
> > + struct job_list_entry *entry,
> > + struct subtest_list *subtests,
> > + struct json_object *tests)
> > +{
> > + char piglit_name[256];
> > + char dynamic_piglit_name[256];
> > + size_t i, k;
> > +
> > + if (settings->prune_mode == PRUNE_KEEP_ALL)
> > + return;
> > +
> > for (i = 0; i < subtests->size; i++) {
> > - if (subtests->subs[i].dynamic_size) {
> > - generate_piglit_name(binary, subtests->subs[i].name, piglit_name, sizeof(piglit_name));
> > + generate_piglit_name(entry->binary, subtests->subs[i].name, piglit_name, sizeof(piglit_name));
> > +
> > + if (settings->prune_mode == PRUNE_KEEP_DYNAMIC) {
> > + if (subtests->subs[i].dynamic_size)
> > + json_object_object_del(tests, piglit_name);
> > +
> > + continue;
> > + }
> > +
> > + assert(settings->prune_mode == PRUNE_KEEP_SUBTESTS || settings->prune_mode == PRUNE_KEEP_REQUESTED);
> > +
> > + if (settings->prune_mode == PRUNE_KEEP_REQUESTED &&
> > + !result_is_requested(entry, subtests->subs[i].name, NULL)) {
> > json_object_object_del(tests, piglit_name);
> > }
> > + for (k = 0; k < subtests->subs[i].dynamic_size; k++) {
> > + if (settings->prune_mode == PRUNE_KEEP_SUBTESTS ||
> > + (settings->prune_mode == PRUNE_KEEP_REQUESTED &&
> > + !result_is_requested(entry, subtests->subs[i].name, subtests->subs[i].dynamic_names[k]))) {
> > + generate_piglit_name_for_dynamic(piglit_name, subtests->subs[i].dynamic_names[k],
> > + dynamic_piglit_name, sizeof(dynamic_piglit_name));
> > + json_object_object_del(tests, dynamic_piglit_name);
> > + }
> > + }
> > }
> > }
> > @@ -1426,8 +1471,7 @@ static void add_to_totals(const char *binary,
> > for (i = 0; i < subtests->size; i++) {
> > generate_piglit_name(binary, subtests->subs[i].name, piglit_name, sizeof(piglit_name));
> > - if (subtests->subs[i].dynamic_size == 0) {
> > - test = get_or_create_json_object(results->tests, piglit_name);
> > + if (json_object_object_get_ex(results->tests, piglit_name, &test)) {
> > if (!json_object_object_get_ex(test, "result", &resultobj)) {
> > fprintf(stderr, "Warning: No results set for %s\n", piglit_name);
> > return;
> > @@ -1441,17 +1485,18 @@ static void add_to_totals(const char *binary,
> > for (k = 0; k < subtests->subs[i].dynamic_size; k++) {
> > generate_piglit_name_for_dynamic(piglit_name, subtests->subs[i].dynamic_names[k],
> > dynamic_piglit_name, sizeof(dynamic_piglit_name));
> > - test = get_or_create_json_object(results->tests, dynamic_piglit_name);
> > - if (!json_object_object_get_ex(test, "result", &resultobj)) {
> > - fprintf(stderr, "Warning: No results set for %s\n", dynamic_piglit_name);
> > - return;
> > +
> > + if (json_object_object_get_ex(results->tests, dynamic_piglit_name, &test)) {
> > + if (!json_object_object_get_ex(test, "result", &resultobj)) {
> > + fprintf(stderr, "Warning: No results set for %s\n", dynamic_piglit_name);
> > + return;
> > + }
> > + result = json_object_get_string(resultobj);
> > + add_result_to_totals(emptystrtotal, result);
> > + add_result_to_totals(roottotal, result);
> > + add_result_to_totals(binarytotal, result);
> > }
> > - result = json_object_get_string(resultobj);
> > - add_result_to_totals(emptystrtotal, result);
> > - add_result_to_totals(roottotal, result);
> > - add_result_to_totals(binarytotal, result);
> > }
> > -
> > }
> > }
> > @@ -1483,9 +1528,9 @@ static bool parse_test_directory(int dirfd,
> > goto parse_output_end;
> > }
> > - prune_subtests_with_dynamic_subtests(entry->binary, &subtests, results->tests);
> > -
> > override_results(entry->binary, &subtests, results->tests);
> > + prune_subtests(settings, entry, &subtests, results->tests);
> > +
> > add_to_totals(entry->binary, &subtests, results);
> > parse_output_end:
> > diff --git a/runner/settings.c b/runner/settings.c
> > index a7a12f50..cd64b964 100644
> > --- a/runner/settings.c
> > +++ b/runner/settings.c
> > @@ -29,6 +29,7 @@ enum {
> > OPT_ENABLE_CODE_COVERAGE,
> > OPT_COV_RESULTS_PER_TEST,
> > OPT_VERSION,
> > + OPT_PRUNE_MODE,
> > OPT_HELP = 'h',
> > OPT_NAME = 'n',
> > OPT_DRY_RUN = 'd',
> > @@ -65,6 +66,18 @@ static struct {
> > { 0, 0 },
> > };
> > +static struct {
> > + int value;
> > + const char *name;
> > +} prune_modes[] = {
> > + { PRUNE_KEEP_DYNAMIC, "keep-dynamic-subtests" },
> > + { PRUNE_KEEP_DYNAMIC, "keep-dynamic" },
> > + { PRUNE_KEEP_SUBTESTS, "keep-subtests" },
> > + { PRUNE_KEEP_ALL, "keep-all" },
> > + { PRUNE_KEEP_REQUESTED, "keep-requested" },
> > + { 0, 0 },
> > +};
> > +
> > static bool set_log_level(struct settings* settings, const char *level)
> > {
> > typeof(*log_levels) *it;
> > @@ -103,6 +116,20 @@ static bool set_abort_condition(struct settings* settings, const char *cond)
> > return false;
> > }
> > +static bool set_prune_mode(struct settings* settings, const char *mode)
> > +{
> > + typeof(*prune_modes) *it;
> > +
> > + for (it = prune_modes; it->name; it++) {
> > + if (!strcmp(mode, it->name)) {
> > + settings->prune_mode = it->value;
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > +
> > static bool parse_abort_conditions(struct settings *settings, const char *optarg)
> > {
> > char *dup, *origdup, *p;
> > @@ -239,6 +266,19 @@ static const char *usage_str =
> > " (longer) filter list means the test result should\n"
> > " change. KERN_NOTICE dmesg level is treated as warn,\n"
> > " unless overridden with --dmesg-warn-level.\n"
> > + " --prune-mode <mode> Control reporting of dynamic subtests by selecting test\n"
> > + " results that are removed from the final results set.\n"
> > + " Possible options:\n"
> > + " keep-dynamic-subtests - Remove subtests that have dynamic\n"
> > + " subtests. (default)\n"
> > + " keep-dynamic - Alias for the above\n"
> > + " keep-subtests - Remove dynamic subtests,\n"
> > + " leaving just the parent subtest.\n"
> > + " keep-all - Don't remove anything\n"
> > + " keep-requested - Remove reported results that are\n"
> > + " not in the requested test set.\n"
> > + " Useful when you have a hand-written\n"
> > + " testlist.\n"
> > " -b, --blacklist FILENAME\n"
> > " Exclude all test matching to regexes from FILENAME\n"
> > " (can be used more than once)\n"
> > @@ -423,6 +463,7 @@ bool parse_options(int argc, char **argv,
> > {"use-watchdog", no_argument, NULL, OPT_WATCHDOG},
> > {"piglit-style-dmesg", no_argument, NULL, OPT_PIGLIT_DMESG},
> > {"dmesg-warn-level", required_argument, NULL, OPT_DMESG_WARN_LEVEL},
> > + {"prune-mode", required_argument, NULL, OPT_PRUNE_MODE},
> > {"blacklist", required_argument, NULL, OPT_BLACKLIST},
> > {"list-all", no_argument, NULL, OPT_LIST_ALL},
> > { 0, 0, 0, 0},
> > @@ -521,6 +562,12 @@ bool parse_options(int argc, char **argv,
> > case OPT_DMESG_WARN_LEVEL:
> > settings->dmesg_warn_level = atoi(optarg);
> > break;
> > + case OPT_PRUNE_MODE:
> > + if (!set_prune_mode(settings, optarg)) {
> > + usage("Cannot parse prune mode", stderr);
> > + goto error;
> > + }
> > + break;
> > case OPT_BLACKLIST:
> > if (!parse_blacklist(&settings->exclude_regexes,
> > absolute_path(optarg)))
> > @@ -779,6 +826,7 @@ bool serialize_settings(struct settings *settings)
> > SERIALIZE_LINE(f, settings, use_watchdog, "%d");
> > SERIALIZE_LINE(f, settings, piglit_style_dmesg, "%d");
> > SERIALIZE_LINE(f, settings, dmesg_warn_level, "%d");
> > + SERIALIZE_LINE(f, settings, prune_mode, "%d");
> > SERIALIZE_LINE(f, settings, test_root, "%s");
> > SERIALIZE_LINE(f, settings, results_path, "%s");
> > SERIALIZE_LINE(f, settings, enable_code_coverage, "%d");
> > @@ -830,6 +878,7 @@ bool read_settings_from_file(struct settings *settings, FILE *f)
> > PARSE_LINE(settings, name, val, use_watchdog, numval);
> > PARSE_LINE(settings, name, val, piglit_style_dmesg, numval);
> > PARSE_LINE(settings, name, val, dmesg_warn_level, numval);
> > + PARSE_LINE(settings, name, val, prune_mode, numval);
> > PARSE_LINE(settings, name, val, test_root, val ? strdup(val) : NULL);
> > PARSE_LINE(settings, name, val, results_path, val ? strdup(val) : NULL);
> > PARSE_LINE(settings, name, val, enable_code_coverage, numval);
> > diff --git a/runner/settings.h b/runner/settings.h
> > index bbd965d2..6ae64695 100644
> > --- a/runner/settings.h
> > +++ b/runner/settings.h
> > @@ -24,6 +24,13 @@ _Static_assert(ABORT_ALL == (ABORT_TAINT | ABORT_LOCKDEP | ABORT_PING), "ABORT_A
> > #define GCOV_RESET GCOV_DIR "/reset"
> > #define CODE_COV_RESULTS_PATH "code_cov"
> > +enum {
> > + PRUNE_KEEP_DYNAMIC = 0,
> > + PRUNE_KEEP_SUBTESTS,
> > + PRUNE_KEEP_ALL,
> > + PRUNE_KEEP_REQUESTED,
> > +};
> > +
> > struct regex_list {
> > char **regex_strings;
> > GRegex **regexes;
> > @@ -51,6 +58,7 @@ struct settings {
> > char *results_path;
> > bool piglit_style_dmesg;
> > int dmesg_warn_level;
> > + int prune_mode;
> > bool list_all;
> > char *code_coverage_script;
> > bool enable_code_coverage;
>
> --
> ~Swati Sharma
More information about the igt-dev
mailing list