On Fri, Apr 29, 2022 at 1:01 AM David Gow <davidgow@xxxxxxxxxx> wrote: > > On Wed, Apr 27, 2022 at 11:06 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > > > On Tue, Apr 26, 2022 at 8:56 PM David Gow <davidgow@xxxxxxxxxx> 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@xxxxxxxxxx I also added on a new patch to fix the type confusion in the debugfs code (using bool instead of enum kunit_status).
