On Fri, 31 Mar 2023 at 06:21, 'Daniel Latypov' via KUnit Development kunit-dev@googlegroups.com wrote:
I've got a few minor comments below, but this otherwise looks good. I like the idea of testing knuit_fail_current_test().
On Thu, Mar 30, 2023 at 3:05 PM Rae Moar rmoar@google.com wrote:
+static void kunit_current_kunit_test_field(struct kunit *test) +{
struct kunit *current_test;/* Check to ensure the result of current->kunit_test* is equivalent to current test.*/current_test = current->kunit_test;KUNIT_EXPECT_PTR_EQ(test, test, current_test);Perhaps we can combine this and the next test case down to static void kunit_current_test(struct kunit *test) { /* There are two different ways of getting the current test */ KUNIT_EXPECT_PTR_EQ(test, test, current->kunit_test); KUNIT_EXPECT_PTR_EQ(test, test, kunit_get_current_test()); } ?
Agreed: checking current->kunit_test twice feels a bit odd.
+}
+static void kunit_current_get_current_test(struct kunit *test) +{
struct kunit *current_test1, *current_test2;/* Check to ensure the result of kunit_get_current_test()* is equivalent to current test.*/current_test1 = kunit_get_current_test();KUNIT_EXPECT_PTR_EQ(test, test, current_test1);/* Check to ensure the result of kunit_get_current_test()* is equivalent to current->kunit_test.*/current_test2 = current->kunit_test;KUNIT_EXPECT_PTR_EQ(test, current_test1, current_test2);+}
+static void kunit_current_fail_current_test(struct kunit *test) +{
struct kunit fake;/* Initialize fake test and set as current->kunit_test. */Nit: I think the code is self-explanatory enough that we can drop this comment.
kunit_init_test(&fake, "fake test", NULL);KUNIT_EXPECT_EQ(test, fake.status, KUNIT_SUCCESS);current->kunit_test = &fake;/* Fail current test and expect status of fake test to be failed. */Nit: I think this comment could also be dropped or maybe shortened to kunit_fail_current_test("This should make `fake` fail");
or /* Now kunit_fail_current_test() should modify `fake`, not `test` */ kunit_fail_current_test("This should make `fake` fail");
kunit_fail_current_test("This test is supposed to fail.");KUNIT_EXPECT_EQ(test, fake.status, (enum kunit_status)KUNIT_FAILURE);Hmm, should we try calling kunit_cleanup(&fake); ?
Right now this does resource cleanups, but we might have other state to cleanup for our `fake` test object in the future.
I could go either way here. We currently don't do this with the other status tests (kunit_status), only with the resource ones. But it doesn't hurt to add it...
Daniel
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAGS_qxqNwVcymkG6-8Kv72oZc9aDqjF....