On Fri, Apr 21, 2023 at 1:42 AM David Gow <davidgow@xxxxxxxxxx> wrote: > > Many uses of the KUnit resource system are intended to simply defer > calling a function until the test exits (be it due to success or > failure). The existing kunit_alloc_resource() function is often used for > this, but was awkward to use (requiring passing NULL init functions, etc), > and returned a resource without incrementing its reference count, which > -- while okay for this use-case -- could cause problems in others. > > Instead, introduce a simple kunit_add_action() API: a simple function > (returning nothing, accepting a single void* argument) can be scheduled > to be called when the test exits. Deferred actions are called in the > opposite order to that which they were registered. > > This mimics the devres API, devm_add_action(), and also provides > kunit_remove_action(), to cancel a deferred action, and > kunit_release_action() to trigger one early. Apologies for the delayed bikeshedding. I think mimicking the devres API is a better idea than kunit_defer() and friends. But I can't help but think this still isn't the best name. I personally would have no idea what `kunit_release_action()` does without looking it up. I feel like `kunit_add_cleanup()` probably works better for a unit test framework. I think `kunit_remove_cleanup()` is fine and `kunit_release_cleanup()` is questionably ok. Instead of `release`, maybe it should be `kunit_trigger_cleanup()` or more verbosely, something like `kunit_early_trigger_cleanup()`. I tried to look for equivalents in other languages/frameworks: * Rust and C++ rely on RAII, don't think they have equivalents in testing libs * Python has `self.addCleanup()`, https://docs.python.org/3/library/unittest.html#unittest.TestCase.addCleanup * Go has `t.Cleanup()`, https://pkg.go.dev/testing#T.Cleanup * Looking at Zig since it also has a `defer`, I guess they just use that, I don't see anything in https://ziglang.org/documentation/master/std/#A;std:testing * I know nothing about JUnit, but a quick search seems like they rely on @After and @AfterClass annotations, https://junit.org/junit4/javadoc/4.12/org/junit/After.html * I know even less about HUnit, but it looks like it relies on wrapping things via the IO monad, https://hackage.haskell.org/package/HUnit-1.6.2.0/docs/Test-HUnit-Base.html#t:AssertionPredicate * Since we were inspired by TAP, I tried to look at Perl, but didn't immediately see anything that looked equivalent, https://metacpan.org/pod/Test::Most > > This is implemented as a resource under the hood, so the ordering > between resource cleanup and deferred functions is maintained. > > Signed-off-by: David Gow <davidgow@xxxxxxxxxx> > --- <snip> > diff --git a/include/kunit/resource.h b/include/kunit/resource.h > index c0d88b318e90..6db28cd43e9b 100644 > --- a/include/kunit/resource.h > +++ b/include/kunit/resource.h > @@ -387,4 +387,80 @@ static inline int kunit_destroy_named_resource(struct kunit *test, > */ > void kunit_remove_resource(struct kunit *test, struct kunit_resource *res); > > + > +/** > + * kunit_add_action() - Defer an 'action' (function call) until the test ends. > + * @test: Test case to associate the action with. > + * @func: The function to run on test exit > + * @ctx: Data passed into @func > + * > + * Defer the execution of a function until the test exits, either normally or > + * due to a failure. @ctx is passed as additional context. All functions > + * registered with kunit_add_action() will execute in the opposite order to that > + * they were registered in. > + * > + * This is useful for cleaning up allocated memory and resources. Re renaming to kunit_add_cleanup(), I think this makes writing the comment easier. E.g. - kunit_add_action() - Defer an 'action' (function call) until the test ends. + kunit_add_cleanup() - Call a function when the test ends. + ... + This is useful for cleaning up allocated memory and resources. Daniel
