From: Daniel Latypov <dlatypov@google.com>
To: David Gow <davidgow@google.com>
Cc: Brendan Higgins <brendanhiggins@google.com>,
Rae Moar <rmr167@gmail.com>,
Shuah Khan <skhan@linuxfoundation.org>,
kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/4] kunit: Don't crash if no parameters are generated
Date: Wed, 27 Oct 2021 11:59:55 -0700 [thread overview]
Message-ID: <CAGS_qxqbJd+8U4TQCusmcNND0vdUCF2EJqBqXhtx3NBt4KAAGQ@mail.gmail.com> (raw)
In-Reply-To: <20211027013702.2039566-3-davidgow@google.com>
On Tue, Oct 26, 2021 at 6:37 PM David Gow <davidgow@google.com> wrote:
>
> It's possible that a parameterised test could end up with zero
> parameters. At the moment, the test function will nevertheless be called
> with NULL as the parameter. Instead, don't try to run the test code, and
> just mark the test as SKIPped.
>
> Reported-by: Daniel Latypov <dlatypov@google.com>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
> lib/kunit/test.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 3bd741e50a2d..e028d98e4f5b 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -500,7 +500,10 @@ int kunit_run_tests(struct kunit_suite *suite)
> kunit_print_subtest_start(suite);
>
> kunit_suite_for_each_test_case(suite, test_case) {
> - struct kunit test = { .param_value = NULL, .param_index = 0 };
> + /* The initial param value is nonzero, as we want
> + * non-parametrised tests to run once.
> + */
> + struct kunit test = { .param_value = (void *)-1, .param_index = 0 };
(Not a strong preference)
Hmm, I'd slightly prefer we don't set a dummy value of -1 for this.
I personally think something like this is a bit less subtle:
/* Run non-parameterised tests once */
while (!test_case->generate_param || test.param_value) {
if (!test_case->generate_param) break;
}
Alternatively, we don't need to share the loop
if (!test_case->generate_param) {
kunit_run_case_catch_errors(suite, test_case, &test);
kunit_update_stats(¶m_stats, test.status);
} else while (test_param.value) {
kunit_run_case_catch_errors(suite, test_case, &test);
kunit_update_stats(¶m_stats, test.status);
/* print subtest and advance next param */
}
}
> struct kunit_result_stats param_stats = { 0 };
> test_case->status = KUNIT_SKIPPED;
>
> @@ -510,7 +513,7 @@ int kunit_run_tests(struct kunit_suite *suite)
> test.param_value = test_case->generate_params(NULL, param_desc);
> }
>
> - do {
> + while (test.param_value) {
> kunit_run_case_catch_errors(suite, test_case, &test);
>
> if (test_case->generate_params) {
> @@ -530,11 +533,12 @@ int kunit_run_tests(struct kunit_suite *suite)
> param_desc[0] = '\0';
> test.param_value = test_case->generate_params(test.param_value, param_desc);
> test.param_index++;
> - }
> + } else
> + test.param_value = NULL;
>
> kunit_update_stats(¶m_stats, test.status);
>
> - } while (test.param_value);
> + }
>
> kunit_print_test_stats(&test, param_stats);
>
> --
> 2.33.0.1079.g6e70778dc9-goog
>
next prev parent reply other threads:[~2021-10-27 19:00 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-27 1:36 [PATCH v2 1/4] kunit: tool: Do not error on tests without test plans David Gow
2021-10-27 1:37 ` [PATCH v2 2/4] kunit: tool: Report an error if any test has no subtests David Gow
2021-10-27 1:37 ` [PATCH v2 3/4] kunit: Don't crash if no parameters are generated David Gow
2021-10-27 18:59 ` Daniel Latypov [this message]
2021-10-27 1:37 ` [PATCH v2 4/4] kunit: Report test parameter results as (K)TAP subtests David Gow
2021-10-27 22:12 ` [PATCH v2 1/4] kunit: tool: Do not error on tests without test plans Daniel Latypov
2021-10-27 23:02 ` David Gow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAGS_qxqbJd+8U4TQCusmcNND0vdUCF2EJqBqXhtx3NBt4KAAGQ@mail.gmail.com \
--to=dlatypov@google.com \
--cc=brendanhiggins@google.com \
--cc=davidgow@google.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=rmr167@gmail.com \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).