From mboxrd@z Thu Jan 1 00:00:00 1970 From: Noah Massey Subject: Re: [PATCH v3 3/3] lib/test_crc: Add test cases for crc calculation Date: Wed, 18 Jul 2018 14:30:27 -0400 Message-ID: References: <20180717145525.50852-1-colyli@suse.de> <20180717145525.50852-4-colyli@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: andy.shevchenko@gmail.com Cc: Coly Li , linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org, linux-block@vger.kernel.org, gregkh@linuxfoundation.org, torvalds@linux-foundation.org, tglx@linutronix.de, kstewart@linuxfoundation.org List-Id: linux-bcache@vger.kernel.org On Tue, Jul 17, 2018 at 4:59 PM Andy Shevchenko wrote: > On Tue, Jul 17, 2018 at 9:51 PM, Noah Massey wrote: > > On Tue, Jul 17, 2018 at 10:56 AM Coly Li 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