On Thu, Sep 07, 2017 at 09:49:42AM +0300, Abdiel Janulgue wrote: > Check support before executing test. > v2: Skip test only if intel_l3_parity tool tells us to skip. (Petri) > > bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101650 > Cc: Petri Latvala <petri.latvala@xxxxxxxxx> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx> > --- > tests/tools_test.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tests/tools_test.c b/tests/tools_test.c > index ccd165d..ebd4a9d 100644 > --- a/tests/tools_test.c > +++ b/tests/tools_test.c > @@ -89,7 +89,8 @@ igt_main > igt_system_cmd(exec_return, > "../tools/intel_l3_parity -r 0 -b 0 " > "-s 0 -e"); > - igt_assert(exec_return == IGT_EXIT_SUCCESS); > + igt_skip_on_f(exec_return == IGT_EXIT_SKIP, > + "intel_l3_parity not supported\n"); > Asserting that exec_return equals IGT_EXIT_SUCCESS would be nice here, after the skip_on_f. Btw, use igt_assert_eq for that assert instead of using == so we get the values in the failure log. > igt_system_cmd(exec_return, > "../tools/intel_l3_parity -l | " > @@ -101,13 +102,14 @@ igt_main > &val); > igt_assert(val == 1); > } else { > - igt_fail(IGT_EXIT_FAILURE); > + igt_skip("intel_l3_parity not supported\n"); > } > This part still causes problems that are misreported. In this pipeline # intel_l3_parity -l | grep whatever the exit status of the shell is the exit status of grep, not intel_l3_parity. Not to mention we don't get to see what intel_l3_parity even printed here. Make that call to igt_system_cmd just call intel_l3_parity -l, and check for the "Row blah, Bank blah" string in your check_cmd_return value function (needs to also be renamed and the comment explaining it reworded, that makes no sense to me now and will make less sense with that change). Make sure you account for the log buffer possibly having more than just that one line, maybe like this: -- begin untested code -- static bool find_the_line(const char *haystack, void *data) { int *val = data; const char *needle = "Row blah etc"; if (strstr(haystack, needle)) { *val = 1; return false; } return true; } /* pseudo in the subtest */ igt_system_cmd(exec_return, "path/intel_l3_parity -l"); if (succeeded) { igt_log_buffer_inspect(find_the_line, &val); igt_assert_eq(val, 1); } -- end untested code -- Bonus points for making the find_the_line function more reusable by having the needle be specifiable. > igt_system_cmd(exec_return, > "../tools/intel_l3_parity -r 0 -b 0 " > "-s 0 -e"); > - igt_assert(exec_return == IGT_EXIT_SUCCESS); > + igt_skip_on_f(exec_return == IGT_EXIT_SKIP, > + "intel_l3_parity not supported\n"); > The "intel_l3_parity not supported" should be reworded everywhere to state that the particular attempted operation is not supported. Same here for asserting EXIT_SUCCESS after checking for EXIT_SKIP -- Petri Latvala _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
