Create test suite called "kunit_current" to add test coverage for the use of current->kunit_test, which returns the current KUnit test.
Add three test cases: - kunit_current_kunit_test_field to test the use of current->kunit_test.
- kunit_current_get_current_test to test the method kunit_get_current_test(), which utilizes current->kunit_test.
- kunit_current_fail_current_test to test the method kunit_fail_current_test(), which utilizes current->kunit_test.
Signed-off-by: Rae Moar rmoar@google.com --- lib/kunit/kunit-test.c | 61 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index b63595d3e241..91984b92c916 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -6,6 +6,7 @@ * Author: Brendan Higgins brendanhiggins@google.com */ #include <kunit/test.h> +#include <kunit/test-bug.h>
#include "try-catch-impl.h"
@@ -532,7 +533,65 @@ static struct kunit_suite kunit_status_test_suite = { .test_cases = kunit_status_test_cases, };
+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); +} + +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. */ + 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. */ + kunit_fail_current_test("This test is supposed to fail."); + KUNIT_EXPECT_EQ(test, fake.status, (enum kunit_status)KUNIT_FAILURE); + + /* Reset current->kunit_test to current test. */ + current->kunit_test = test; +} + +static struct kunit_case kunit_current_test_cases[] = { + KUNIT_CASE(kunit_current_kunit_test_field), + KUNIT_CASE(kunit_current_get_current_test), + KUNIT_CASE(kunit_current_fail_current_test), + {} +}; + +static struct kunit_suite kunit_current_test_suite = { + .name = "kunit_current", + .test_cases = kunit_current_test_cases, +}; + kunit_test_suites(&kunit_try_catch_test_suite, &kunit_resource_test_suite, - &kunit_log_test_suite, &kunit_status_test_suite); + &kunit_log_test_suite, &kunit_status_test_suite, + &kunit_current_test_suite);
MODULE_LICENSE("GPL v2");
base-commit: 7232282dd47cce6a780c9414bd9baccf232c7686
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()); } ?
+}
+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.
Daniel
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....
On Thu, Mar 30, 2023 at 6:21 PM '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()); } ?
Hi Daniel!
Yes, I would be happy to combine these for v2. I might want to alter that proposed comment slightly. "Two different ways" seems a bit unclear to me. Maybe: Check results of both current->kunit_test and kunit_get_current_test() are equivalent to current test. What do you think? I might send out a v2 with a proposed comment.
+}
+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.
I agree the "initialize fake test" comment is self-explanatory. But if we keep the comment regarding resetting the current test, I think we should mark when we set the test as a fake with a comment as well.
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");
This first option seems good to me.
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 would be fine to add this here if it is wanted.
Thanks Daniel for the comments!
Rae
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....
On Mon, Apr 3, 2023 at 12:31 PM Rae Moar rmoar@google.com wrote:
On Thu, Mar 30, 2023 at 6:21 PM '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()); } ?
Hi Daniel!
Yes, I would be happy to combine these for v2. I might want to alter that proposed comment slightly. "Two different ways" seems a bit unclear to me. Maybe: Check results of both current->kunit_test and kunit_get_current_test() are equivalent to current test. What do you think? I might send out a v2 with a proposed comment.
What you went with in v2 works for me. I'll take a look at the other changes in v2.
Thanks! Daniel
linux-kselftest-mirror@lists.linaro.org
-
Daniel Latypov -
David Gow -
Rae Moar