On Fri, Apr 29, 2022 at 1:01 AM David Gow davidgow@google.com wrote:
On Wed, Apr 27, 2022 at 11:06 AM Daniel Latypov dlatypov@google.com wrote:
On Tue, Apr 26, 2022 at 8:56 PM David Gow davidgow@google.com wrote:
static size_t kunit_suite_counter = 1;
-static void kunit_print_suite_end(struct kunit_suite *suite) +static void kunit_print_suite_end(struct kunit_suite *suite, int init_err)
A part of me feels that it'd be nicer to have the init_err be part of struct kunit_suite, and have kunit_suite_has_succeeded() take it into account. It could go either way, though -- WDYT?
Yeah, passing it around as a parameter felt a bit icky. But I think adding it in as a field feels worse.
Personally, I don't have a problem with having it as a field (other than the memory usage, which shouldn't be so much as to cause problems). It seems that the suite result is logically part of the suite, and given that status_comment is in struct kunit_suite and there's a kunit_status field in kunit_case, having it as a field in the suite seems the most logically consistent thing to do.
Another thought: perhaps have this function take a `kunit_status` parameter instead? Moving the ?: expression below out into the caller isn't that bad, imo.
It still doesn't solve the fact that kunit_suite_has_succeeded() no longer tells you if a suite has succeeded or not. If we stick with
I forgot kunit_suite_has_succeeded() is called in the debugfs code. So it looks like we need to track the init error in struct kunit_suite, as you said.
It might be cleaner to just store a status in the suite eventually, but I'll just add the int for now.
Sending a v2 series here: https://lore.kernel.org/linux-kselftest/20220429181259.622060-1-dlatypov@goo... I also added on a new patch to fix the type confusion in the debugfs code (using bool instead of enum kunit_status).