> -----Original Message----- > From: Paul Elder on Thursday, June 15, 2017 11:27 AM > On 06/15/2017 03:53 AM, Bird, Timothy wrote: > >> -----Original Message----- > >> From: Paul Elder on Wednesday, June 14, 2017 12:12 PM > >> On 06/14/2017 04:00 AM, Bird, Timothy wrote: > >>>> -----Original Message----- > >>>> From: Greg Kroah-Hartman [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > >>>> Sent: Monday, June 12, 2017 3:57 PM > >>>> To: shuah@xxxxxxxxxx > >>>> Cc: Paul Elder <paul.elder@xxxxxxxx>; linux-kselftest@xxxxxxxxxxxxxxx; > >> Greg > >>>> Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Bird, Timothy > >>>> <Tim.Bird@xxxxxxxx>; Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx> > >>>> Subject: [PATCH v2 2/4] kselftest: membarrier: convert to TAP13 > output > >>>> > >>>> From: Paul Elder <paul.elder@xxxxxxxx> > >>>> > >>>> Make the membarrier test output in the TAP13 format by using the > >>>> TAP13 output functions defined in kselftest.h > >>>> > >>>> Signed-off-by: Paul Elder <paul.elder@xxxxxxxx> > >>>> Signed-off-by: Alice Ferrazzi <alice.ferrazzi@xxxxxxxxx> > >>>> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > >>>> --- > >>>> v2: Just use the standard function names, no _tap suffix - Alice > >>>> > >>>> > >>>> .../testing/selftests/membarrier/membarrier_test.c | 35 > >> ++++++++++++---- > >>>> ------ > >>>> 1 file changed, 19 insertions(+), 16 deletions(-) > >>>> > >>>> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c > >>>> b/tools/testing/selftests/membarrier/membarrier_test.c > >>>> index 535f0fef4d0b..cae8c984dfb0 100644 > >>>> --- a/tools/testing/selftests/membarrier/membarrier_test.c > >>>> +++ b/tools/testing/selftests/membarrier/membarrier_test.c > >>>> @@ -21,36 +21,42 @@ static int sys_membarrier(int cmd, int flags) > >>>> static enum test_membarrier_status test_membarrier_cmd_fail(void) > >>>> { > >>>> int cmd = -1, flags = 0; > >>>> + const char *test_name = "membarrier command fail"; > >>>> > >>>> if (sys_membarrier(cmd, flags) != -1) { > >>>> - printf("membarrier: Wrong command should fail but > >>>> passed.\n"); > >>>> + ksft_test_result_fail(test_name); > >>> > >>> As written, the conversion to TAP loses this explanation of the error. > >>> > >>> If you follow my recommendations for patch 1/4, then this becomes: > >>> ksft_fail(test_name, "membarrier: Wrong command should fail but > >> passed.") > >> Looking at the TAP 13 specification, I can see your point: where the test > >> description is invariant whereas the reason for failure should be printed > >> separately. However, as I have pointed out in my reply for patch 1/4, I > >> think that the "reason" output should be outputted with the yaml. I do > see > >> that the failure reason should be tied to the test, but then wouldn't the > >> yaml closing become messy if the test wants to output more yaml after > the > >> failure reason? > > > > Yes. That's why I showed 3 different examples of usage: > > > > 1) failure with no extra data: > > kfst_test_result_fail(test_name, NULL); > > > > 2) failure with one line of extra data: > > kfst_test_result_fail(test_name, "reason string"); > > Note this puts the reason string into YAML block with only > > one line of content. > > > > 3) failure with more lines of extra data: > > ksft_test_result_fail(test_name); > > ksft_yaml(more data - format TBD); > > > > My intent was that a caller would use only one of these, and not call > > ksft_yaml() if they had already included extra data in the > kfst_test_result_fail() call. > I think that is good, but two of the three scenarios that you have given > don't have the reason string included in the ksft_test_result_fail() call. > > > However, I can see how this would be confusing and possibly error prone. > Indeed. Error prone indeed. In a bit of unintended humor, I actually messed up the calls in my own example. In case number three I missed the NULL pointer argument. Oops. So yeah, let's make this easy enough so that even I can do it. :-) -- Tim -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
