[igt-dev] [PATCH i-g-t 5/9] runner: Abort the run when test exits with IGT_EXIT_ABORT
Petri Latvala
petri.latvala at intel.com
Fri Feb 14 12:16:50 UTC 2020
On Wed, Feb 12, 2020 at 03:22:32PM +0200, Arkadiusz Hiler wrote:
> Now that the IGT tests have a mechanism for signaling broken testing
> conditions we can stop the run on the first test that has noticed it,
> and possibly has triggered that state.
>
> Traditionally run would have continued with that test failing and the
> side effects would trickle down into the other tests causing a lot of
> skip/fails.
>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
> runner/executor.c | 3 +
> .../aborted-after-a-test/reference.json | 6 +
> .../aborted-on-boot/reference.json | 6 +
> .../dmesg-escapes/reference.json | 3 +
> .../dmesg-results/reference.json | 5 +
> .../reference.json | 3 +
> .../reference.json | 3 +
> .../dmesg-warn-level/reference.json | 3 +
> .../reference.json | 3 +
> .../dynamic-subtests/reference.json | 3 +
> .../reference.json | 5 +
> .../json_tests_data/normal-run/reference.json | 5 +
> .../reference.json | 4 +
> .../notrun-results/reference.json | 5 +
> .../piglit-style-dmesg/reference.json | 5 +
> .../unprintable-characters/reference.json | 5 +-
> .../warnings-with-dmesg-warns/reference.json | 5 +
> .../json_tests_data/warnings/reference.json | 5 +
Ahh, the joys of top-down testing.
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 105ec887..5e72db9a 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -645,6 +645,16 @@ static void process_dynamic_subtest_output(const char *piglit_name,
> &dynresulttext, &dyntime,
> dyn_result_idx < 0 ? NULL : matches.items[dyn_result_idx].where,
> dynend);
> +
> + if (!strcmp(dynresulttext, "incomplete")) {
> + struct json_object *parent_subtest;
> +
> + if (json_object_object_get_ex(tests, piglit_name, &parent_subtest) &&
> + json_object_object_get_ex(parent_subtest, "result", &parent_subtest) &&
> + !strcmp(json_object_get_string(parent_subtest), "abort"))
> + dynresulttext = "abort";
> + }
> +
Fairly self-explanatory but could use a comment explaining what is
done when and why.
> set_result(current_dynamic_test, dynresulttext);
> set_runtime(current_dynamic_test, dyntime);
> }
> @@ -1078,6 +1088,8 @@ static const char *result_from_exitcode(int exitcode)
> return "pass";
> case IGT_EXIT_INVALID:
> return "skip";
> + case IGT_EXIT_ABORT:
> + return "abort";
> case INCOMPLETE_EXITCODE:
> return "incomplete";
> default:
> @@ -1154,6 +1166,18 @@ static void fill_from_journal(int fd,
> }
> }
>
> + if (subtests->size && exitcode == IGT_EXIT_ABORT)
> + {
Your { slipped down from the end of the line.
> + char *last_subtest = subtests->subs[subtests->size - 1].name;
> + char subtest_piglit_name[256];
> + struct json_object *subtest_obj;
> +
> + generate_piglit_name(entry->binary, last_subtest, subtest_piglit_name, sizeof(subtest_piglit_name));
> + subtest_obj = get_or_create_json_object(tests, subtest_piglit_name);
> +
> + set_result(subtest_obj, "abort");
> + }
> +
> if (subtests->size == 0) {
> char *subtestname = NULL;
> char piglit_name[256];
> @@ -1297,6 +1321,7 @@ static struct json_object *get_totals_object(struct json_object *totals,
> json_object_object_add(obj, "dmesg-warn", json_object_new_int(0));
> json_object_object_add(obj, "skip", json_object_new_int(0));
> json_object_object_add(obj, "incomplete", json_object_new_int(0));
> + json_object_object_add(obj, "abort", json_object_new_int(0));
> json_object_object_add(obj, "timeout", json_object_new_int(0));
> json_object_object_add(obj, "notrun", json_object_new_int(0));
> json_object_object_add(obj, "fail", json_object_new_int(0));
> diff --git a/runner/runner_tests.c b/runner/runner_tests.c
> index dd590f33..701da1a5 100644
> --- a/runner/runner_tests.c
> +++ b/runner/runner_tests.c
> @@ -28,9 +28,16 @@ static const char testdatadir[] = TESTDATA_DIRECTORY;
> * that test binaries without subtests should still be counted as one
> * for this macro.
> */
> -#define NUM_TESTDATA_SUBTESTS 6
> +#define NUM_TESTDATA_SUBTESTS 15
> +#define NUM_TESTDATA_ABORT_SUBTESTS 9
> /* The total number of test binaries in runner/testdata/ */
> -#define NUM_TESTDATA_BINARIES 4
> +#define NUM_TESTDATA_BINARIES 8
> +
> +static void igt_assert_no_result_for(struct json_object *tests, const char* testname)
> +{
> + struct json_object *obj;
> + igt_assert(!json_object_object_get_ex(tests, testname, &obj));
> +}
>
> static const char *igt_get_result(struct json_object *tests, const char* testname)
> {
> @@ -941,6 +948,7 @@ igt_main
> struct execute_state state;
> const char *argv[] = { "runner",
> "--dry-run",
> + "-x", "^abort",
> testdatadir,
> dirname,
> };
> @@ -951,7 +959,7 @@ igt_main
> igt_assert(initialize_execute_state(&state, settings, list));
> igt_assert_eq(state.next, 0);
> igt_assert(state.dry);
> - igt_assert_eq(list->size, NUM_TESTDATA_SUBTESTS);
> + igt_assert_eq(list->size, NUM_TESTDATA_SUBTESTS - NUM_TESTDATA_ABORT_SUBTESTS);
>
> igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> "Dry run initialization didn't create the results directory.\n");
> @@ -972,7 +980,7 @@ igt_main
> igt_assert(initialize_execute_state_from_resume(dirfd, &state, settings, list));
> igt_assert_eq(state.next, 0);
> igt_assert(!state.dry);
> - igt_assert_eq(list->size, NUM_TESTDATA_SUBTESTS);
> + igt_assert_eq(list->size, NUM_TESTDATA_SUBTESTS - NUM_TESTDATA_ABORT_SUBTESTS);
> /* initialize_execute_state_from_resume() closes the dirfd */
> igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> "Dry run resume somehow deleted the results directory.\n");
> @@ -1499,7 +1507,7 @@ igt_main
> struct execute_state state;
> struct json_object *results, *tests;
> const char *argv[] = { "runner",
> - "-t", "dynamic",
> + "-t", "^dynamic$",
> testdatadir,
> dirname,
> };
> @@ -1528,6 +1536,291 @@ igt_main
> }
> }
>
> + igt_subtest_group {
> + struct job_list *list = malloc(sizeof(*list));
> + volatile int dirfd = -1;
> + char dirname[] = "tmpdirXXXXXX";
> +
> + igt_fixture {
> + igt_require(mkdtemp(dirname) != NULL);
> + rmdir(dirname);
> +
> + init_job_list(list);
> + }
> +
> + igt_subtest("execute-abort-simple") {
> + struct execute_state state;
> + struct json_object *results, *tests;
> + const char *argv[] = { "runner",
> + "-t", "^abort-simple$",
> + testdatadir,
> + dirname,
> + };
> +
> + igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, settings));
> + igt_assert(create_job_list(list, settings));
> + igt_assert(initialize_execute_state(&state, settings, list));
> + igt_assert(!execute(&state, settings, list)); /* false = signal abort */
> +
> + igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> + "Execute didn't create the results directory\n");
> + igt_assert_f((results = generate_results_json(dirfd)) != NULL,
> + "Results parsing failed\n");
> +
> + igt_assert(json_object_object_get_ex(results, "tests", &tests));
> +
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-simple"), "abort");
> +
> + igt_assert_eq(json_object_put(results), 1);
> + }
> +
> + igt_fixture {
> + close(dirfd);
> + clear_directory(dirname);
> + free_job_list(list);
> + }
> + }
> +
> + igt_subtest_group {
> + struct job_list *list = malloc(sizeof(*list));
> + volatile int dirfd = -1;
> +
> +
Extra empty line here.
> + for (int multiple = 0; multiple <= 1; ++multiple) {
> + char dirname[] = "tmpdirXXXXXX";
> +
> + igt_fixture {
> + igt_require(mkdtemp(dirname) != NULL);
> + rmdir(dirname);
> +
> + init_job_list(list);
> + }
> +
> + igt_subtest_f("execute-abort%s", multiple ? "-multiple" : "") {
> + struct execute_state state;
> + struct json_object *results, *tests;
> + const char *argv[] = { "runner",
> + "-t", "^abort$",
> + multiple ? "--multiple-mode" : "--sync",
> + testdatadir,
> + dirname,
> + };
> +
> + igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, settings));
> + igt_assert(create_job_list(list, settings));
> + igt_assert(initialize_execute_state(&state, settings, list));
> + igt_assert(!execute(&state, settings, list)); /* false = signal abort */
> +
> + igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> + "Execute didn't create the results directory\n");
> + igt_assert_f((results = generate_results_json(dirfd)) != NULL,
> + "Results parsing failed\n");
> +
> + igt_assert(json_object_object_get_ex(results, "tests", &tests));
> +
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort@a-subtest"), "pass");
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort@b-subtest"), "abort");
> +
> + if (multiple) /* no notrun injection for multiple mode */
> + igt_assert_no_result_for(tests, "igt at abort@c-subtest");
> + else
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort@c-subtest"), "notrun");
> +
> + igt_assert_eq(json_object_put(results), 1);
> + }
> +
> + igt_fixture {
> + close(dirfd);
> + clear_directory(dirname);
> + free_job_list(list);
> + }
> + }
> + }
> +
> + igt_subtest_group {
> + struct job_list *list = malloc(sizeof(*list));
> + volatile int dirfd = -1;
> +
> + for (int multiple = 0; multiple <= 1; ++multiple) {
> + char dirname[] = "tmpdirXXXXXX";
> +
> + igt_fixture {
> + igt_require(mkdtemp(dirname) != NULL);
> + rmdir(dirname);
> +
> + init_job_list(list);
> + }
> +
> + igt_subtest_f("execute-abort-fixture%s", multiple ? "-multiple" : "") {
> + struct execute_state state;
> + struct json_object *results, *tests;
> + const char *argv[] = { "runner", multiple ? "--multiple-mode" : "--sync",
> + "-t", "^abort-fixture$",
> + testdatadir,
> + dirname,
> + };
> +
> + igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, settings));
> + igt_assert(create_job_list(list, settings));
> + igt_assert(initialize_execute_state(&state, settings, list));
> + igt_assert(!execute(&state, settings, list)); /* false = signal abort */
> +
> + igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> + "Execute didn't create the results directory\n");
> + igt_assert_f((results = generate_results_json(dirfd)) != NULL,
> + "Results parsing failed\n");
> +
> + igt_assert(json_object_object_get_ex(results, "tests", &tests));
> +
> + if (multiple) {
> + /*
> + * running the whole binary via -t, no
> + * way of blaming the particular subtest
> + */
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-fixture"), "abort");
> + igt_assert_no_result_for(tests, "igt at abort-fixture@a-subtest");
> + igt_assert_no_result_for(tests, "igt at abort-fixture@b-subtest");
> + } else {
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-fixture@a-subtest"), "abort");
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-fixture@b-subtest"), "notrun");
> + }
> +
> + igt_assert_eq(json_object_put(results), 1);
> + }
> +
> + igt_fixture {
> + close(dirfd);
> + clear_directory(dirname);
> + free_job_list(list);
> + }
> + }
> + }
> +
> + igt_subtest_group {
> + struct job_list *list = malloc(sizeof(*list));
> + volatile int dirfd = -1;
> +
> + for (int multiple = 0; multiple <= 1; ++multiple) {
> + char dirname[] = "tmpdirXXXXXX";
> + char filename[] = "tmplistXXXXXX";
> + const char testlisttext[] = "igt at abort-fixture@b-subtest\n"
> + "igt at abort-fixture@a-subtest\n";
> +
> + igt_fixture {
> + int fd;
> + igt_require((fd = mkstemp(filename)) >= 0);
> + igt_require(write(fd, testlisttext, strlen(testlisttext)) == strlen(testlisttext));
> + close(fd);
> + igt_require(mkdtemp(dirname) != NULL);
> + rmdir(dirname);
> +
> + init_job_list(list);
> + }
> +
> + igt_subtest_f("execute-abort-fixture-testlist%s", multiple ? "-multiple" : "") {
> + struct execute_state state;
> + struct json_object *results, *tests;
> + const char *argv[] = { "runner", multiple ? "--multiple-mode" : "--sync",
> + "--test-list", filename,
> + testdatadir,
> + dirname,
> + };
> +
> + igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, settings));
> + igt_assert(create_job_list(list, settings));
> + igt_assert(initialize_execute_state(&state, settings, list));
> + igt_assert(!execute(&state, settings, list)); /* false = signal abort */
> +
> + igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> + "Execute didn't create the results directory\n");
> + igt_assert_f((results = generate_results_json(dirfd)) != NULL,
> + "Results parsing failed\n");
> +
> + igt_assert(json_object_object_get_ex(results, "tests", &tests));
> +
> + if (multiple) /* multiple mode = no notruns */
> + igt_assert_no_result_for(tests, "igt at abort-fixture@a-subtest");
> + else
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-fixture@a-subtest"), "notrun");
> +
> +
> + igt_assert_eqstr(igt_get_result(tests, "igt at abort-fixture@b-subtest"), "abort");
> +
> + igt_assert_eq(json_object_put(results), 1);
> + }
> +
> + igt_fixture {
> + unlink(filename);
> + close(dirfd);
> + clear_directory(dirname);
> + free_job_list(list);
> + }
> + }
> + }
> +
> + igt_subtest_group {
> + struct job_list *list = malloc(sizeof(*list));
> + volatile int dirfd = -1;
> +
> + for (int multiple = 0; multiple <= 1; ++multiple) {
> + char dirname[] = "tmpdirXXXXXX";
> +
> + igt_fixture {
> + igt_require(mkdtemp(dirname) != NULL);
> + rmdir(dirname);
> +
> + init_job_list(list);
> + }
> +
> + igt_subtest_f("execute-abort-dynamic%s", multiple ? "-multiple" : "") {
> + struct execute_state state;
> + struct json_object *results, *tests;
> + const char *argv[] = { "runner", multiple ? "--multiple-mode" : "--sync",
> + "-t", "^abort-dynamic$",
> + testdatadir,
> + dirname,
> + };
> +
> + igt_assert(parse_options(ARRAY_SIZE(argv), (char**)argv, settings));
> + igt_assert(create_job_list(list, settings));
> + igt_assert(initialize_execute_state(&state, settings, list));
> + igt_assert(!execute(&state, settings, list)); /* false = signal abort */
> +
> + igt_assert_f((dirfd = open(dirname, O_DIRECTORY | O_RDONLY)) >= 0,
> + "Execute didn't create the results directory\n");
> + igt_assert_f((results = generate_results_json(dirfd)) != NULL,
> + "Results parsing failed\n");
> +
> + igt_assert(json_object_object_get_ex(results, "tests", &tests));
> +
> + /* { */
> + /* const char *json_string; */
> + /* json_string = json_object_to_json_string_ext(tests, JSON_C_TO_STRING_PRETTY); */
> + /* printf("****\n%s\n***\n", json_string); */
> + /* } */
New phone, who dis?
Cosmetic editorial comments only, with those addressed,
Reviewed-by: Petri Latvala <petri.latvala at intel.com>
More information about the igt-dev
mailing list