Re: [PATCH v3 3/3] lib/test_crc: Add test cases for crc calculation
From: Noah Massey
Date: Wed Jul 18 2018 - 14:31:07 EST
On Tue, Jul 17, 2018 at 4:59 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Tue, Jul 17, 2018 at 9:51 PM, Noah Massey <noah.massey@xxxxxxxxx> wrote:
> > On Tue, Jul 17, 2018 at 10:56 AM Coly Li <colyli@xxxxxxx> wrote:
>
> >> + if (err == 0)
> >> + pr_info("test_crc: all %d tests passed\n", i);
> >
> > Similar to previous comment: we should not report the number of passed
> > tests, since adding a test would invalidate previous golden output.
> > Also, consider the situation where some tests are conditionally
> > executed depending on kconfig.
>
> We do similar in many test modules and I know at least two that had
> been changed in order to get new test cases.
> Are you proposing to change 'em all?
>
I was proposing that the message should be "test_crc: all tests
passed\n", since that would maintain a static expected output. Upon
further review, parsing minor variations in the messages is simple
enough so if the automated test tools already handle it keeping the
test count in the output is better.
Sorry for the noise,
~ Noah