> -----Original Message-----
> From: Shuah Khan on Wednesday, June 14, 2017 6:24 AM
>
> On 06/13/2017 12:49 PM, Bird, Timothy wrote:
> >> -----Original Message-----
> >> From: Greg Kroah-Hartman on Monday, June 12, 2017 3:57 PM
> >>
> >> Add TAP13 conformat output functions to kselftest.h.
> >>
> >> Also add exit functions that output TAP13 exiting text, as well as
> >> functions to keep track of testing progress.
> >>
> >> 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
> >>
> >> tools/testing/selftests/kselftest.h | 52
> >> ++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 48 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/kselftest.h
> >> b/tools/testing/selftests/kselftest.h
> >> index ef1c80d67ac7..1d874a50d957 100644
> >> --- a/tools/testing/selftests/kselftest.h
> >> +++ b/tools/testing/selftests/kselftest.h
> >> @@ -31,38 +31,82 @@ struct ksft_count {
> >>
> >> static struct ksft_count ksft_cnt;
> >>
> >> +static inline int ksft_test_num(void)
> >> +{
> >> + return ksft_cnt.ksft_pass + ksft_cnt.ksft_fail +
> >> + ksft_cnt.ksft_xfail + ksft_cnt.ksft_xpass +
> >> + ksft_cnt.ksft_xskip;
> >> +}
> >> +
> >> static inline void ksft_inc_pass_cnt(void) { ksft_cnt.ksft_pass++; }
> >> static inline void ksft_inc_fail_cnt(void) { ksft_cnt.ksft_fail++; }
> >> static inline void ksft_inc_xfail_cnt(void) { ksft_cnt.ksft_xfail++; }
> >> static inline void ksft_inc_xpass_cnt(void) { ksft_cnt.ksft_xpass++; }
> >> static inline void ksft_inc_xskip_cnt(void) { ksft_cnt.ksft_xskip++; }
> >>
> >> +static inline void ksft_print_header(void)
> >> +{
> >> + printf("TAP version 13\n");
> >> +}
> >> +
> >> static inline void ksft_print_cnts(void)
> >> {
> >> - printf("Pass: %d Fail: %d Xfail: %d Xpass: %d, Xskip: %d\n",
> >> - ksft_cnt.ksft_pass, ksft_cnt.ksft_fail,
> >> - ksft_cnt.ksft_xfail, ksft_cnt.ksft_xpass,
> >> - ksft_cnt.ksft_xskip);
> >> + printf("1..%d\n", ksft_test_num());
> >> +}
> >> +
> >> +static inline void ksft_test_result_pass(const char *msg)
> >
> > IMHO something shorter like 'ksft_pass' would be better. However, that's
> > a variable name in struct ksft_count. Technically there would be no conflict
> > to have both a function and a structure variable with the same name, but it
> > would be confusing to maintain. I would recommend using the shorter
> > name for the functions, and renaming the internal variables, as shown
> > below.
>
> I would like to leave these as is and add TAP to them. These API are in use
> and the current patch series does it in a way that doesn't force changes to
> the ones that are using the API.
OK - your call. I didn't see any references outside of kselftest.h to
ksft_cnt.ksft_pass (and friends), so I didn't see any external users.
I was looking at Linus's mainline tree v4.12-rc5-12-gef9c135
There don't appear to be any external references in the kselftest
tree either (at least on the master branch). But maybe there
are references on a different branch, or even external to mainline.
ksft_test_result_pass (and friends) are new routines, so I didn't
expect them to have users yet. Maybe I'm missing something.
In any event - I don't want to break any existing code.
>
> >
> > I also think it's not critical to prefix every variable with ksft_ in this
> structure.
> > The variables are only ever accessed using the variable ksft_cnt, so the
> extra
> > prefix is not needed.
> >
> > struct ksft_count {
> > unsigned int pass_count;
> > unsigned int fail_count;
> > unsigned int xfail_count;
> > unsigned int xpass_count;
> > unsigned int xskip_count;
> > };
> >
> > Of course this rename has to flow through all references, like so:
>
> Please see able comment. Would prefer not changing the names.
>
> >
> >> +{
> >> + ksft_cnt.ksft_pass++;
> > ksft_cnt.pass_count++;
> >
> >> + printf("ok %d %s\n", ksft_test_num(), msg);
> >> +}
> >> +
> >> +static inline void ksft_test_result_fail(const char *msg)
> >> +{
> >> + ksft_cnt.ksft_fail++;
> >> + printf("not ok %d %s\n", ksft_test_num(), msg);
> >> +}
> >
> > Same thing here on function name length. Maybe 'kfst_fail'?
> >
> > I think that for all these reporting functions, the first parameters
> > should be const char *desc. It should be the same whether the
> > test passes or fails. Then on a fail, there should be an optional
> > additional parameter indicating the reason for the failure.
> >
> > It's important that the test description be invariant between runs.
> > This can be used as the human-readable identifier for each test.
> > You don't want this message to be a test description on success, but
> > a reason for failure on error. That's not consistent.
> >
> > On success, no other string is required. On failure, it is nice (but optional)
> > to output the reasons for the failure. I would recommend doing this in a
> > YAML block, like so:
> >
> > static inline void ksft_fail(const char *desc, const char *reason)
> > {
> > ksft_cnt.ksft_pass++;
> > printf("ok %d %s\n", ksft_test_num(), desc);
> > if (reason)
> > printf(" ---\n reason: \"%s\"\n ...\n", reason);
> > }
> >
> > Note the leading space for each line of the YAML block, to indent it
> according
> > to the TAP13 protocol.
> >
> > Then a call is either:
> > ksft_fail(desc, NULL);
> > or
> > ksft_fail(desc, "foo index failure")
> >
> >> +
> >> +static inline void ksft_test_result_skip(const char *msg)
> >> +{
> >> + ksft_cnt.ksft_xskip++;
> >> + printf("ok %d # skip %s\n", ksft_test_num(), msg);
> >> }
> >
> > You should add an optional 'reason' for ksft_skip as well.
> >
> >> static inline int ksft_exit_pass(void)
> >> {
> >> + ksft_print_cnts();
> >> exit(KSFT_PASS);
> >> }
> >> +
> >> static inline int ksft_exit_fail(void)
> >> {
> >> + printf("Bail out!\n");
> >> + ksft_print_cnts();
> >> exit(KSFT_FAIL);
> >> }
> >> +
> >> +static inline int ksft_exit_fail_msg(const char *msg)
> >> +{
> >> + printf("Bail out! %s\n", msg);
> >> + ksft_print_cnts();
> >> + exit(KSFT_FAIL);
> >> +}
> >> +
> >> static inline int ksft_exit_xfail(void)
> >> {
> >> + ksft_print_cnts();
> >> exit(KSFT_XFAIL);
> >> }
> >> +
> >> static inline int ksft_exit_xpass(void)
> >> {
> >> + ksft_print_cnts();
> >> exit(KSFT_XPASS);
> >> }
> >> +
> >> static inline int ksft_exit_skip(void)
> >> {
> >> + ksft_print_cnts();
> >> exit(KSFT_SKIP);
> >> }
> >>
> >> --
> >> 2.13.1
> >>
> >
> >
> >
>
��.n��������+%������w��{.n�����{��K ����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
