[PATCH] selftests/harness: pass variant to teardown

[PATCH] selftests/harness: pass variant to teardown

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

FIXTURE_VARIANT data is passed to FIXTURE_SETUP and TEST_F as variant.

In some cases, the variant will change the setup, such that expections
also change on teardown. Also pass variant to FIXTURE_TEARDOWN.

The new FIXTURE_TEARDOWN logic is identical to that in FIXTURE_SETUP,
right above.

Signed-off-by: Willem de Bruijn <willemb@google.com>

---

For one use of this see tentative
tools/testing/selftests/filesystems/selectpoll.c kselftest at
https://github.com/wdebruij/linux-next-mirror/commit/12b4d183ac9140c13606376bb5c6714673daf754
---
 tools/testing/selftests/kselftest_harness.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index f19804df244c..6a27e79278e8 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -283,7 +283,9 @@
 #define FIXTURE_TEARDOWN(fixture_name) \
 	void fixture_name##_teardown( \
 		struct __test_metadata __attribute__((unused)) *_metadata, \
-		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self)
+		FIXTURE_DATA(fixture_name) __attribute__((unused)) *self, \
+		const FIXTURE_VARIANT(fixture_name) \
+			__attribute__((unused)) *variant)
 
 /**
  * FIXTURE_VARIANT(fixture_name) - Optionally called once per fixture
@@ -298,9 +300,9 @@
  *       ...
  *     };
  *
- * Defines type of constant parameters provided to FIXTURE_SETUP() and TEST_F()
- * as *variant*. Variants allow the same tests to be run with different
- * arguments.
+ * Defines type of constant parameters provided to FIXTURE_SETUP(), TEST_F() and
+ * FIXTURE_TEARDOWN as *variant*. Variants allow the same tests to be run with
+ * different arguments.
  */
 #define FIXTURE_VARIANT(fixture_name) struct _fixture_variant_##fixture_name
 
@@ -382,7 +384,7 @@
 		if (!_metadata->passed) \
 			return; \
 		fixture_name##_##test_name(_metadata, &self, variant->data); \
-		fixture_name##_teardown(_metadata, &self); \
+		fixture_name##_teardown(_metadata, &self, variant->data); \
 	} \
 	static struct __test_metadata \
 		      _##fixture_name##_##test_name##_object = { \
-- 
2.29.2.576.ga3fc446d84-goog

Re: [PATCH] selftests/harness: pass variant to teardown

[Jakub Kicinski]

On Thu, 10 Dec 2020 18:10:10 -0500 Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> FIXTURE_VARIANT data is passed to FIXTURE_SETUP and TEST_F as variant.
> 
> In some cases, the variant will change the setup, such that expections
> also change on teardown. Also pass variant to FIXTURE_TEARDOWN.
> 
> The new FIXTURE_TEARDOWN logic is identical to that in FIXTURE_SETUP,
> right above.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

IDK where we want to draw the line between reusing fixtures creating
separate ones, some test cases are completely skipped in the example 
you provide. But in principle the harness change is fine by me:

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

Re: [PATCH] selftests/harness: pass variant to teardown

[Kees Cook]

On Thu, Dec 10, 2020 at 06:10:10PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> FIXTURE_VARIANT data is passed to FIXTURE_SETUP and TEST_F as variant.
> 
> In some cases, the variant will change the setup, such that expections
> also change on teardown. Also pass variant to FIXTURE_TEARDOWN.
> 
> The new FIXTURE_TEARDOWN logic is identical to that in FIXTURE_SETUP,
> right above.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Thanks! This was on my TODO list. :)

Acked-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH] selftests/harness: pass variant to teardown

[Willem de Bruijn]

On Fri, Dec 11, 2020 at 1:15 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Dec 10, 2020 at 06:10:10PM -0500, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > FIXTURE_VARIANT data is passed to FIXTURE_SETUP and TEST_F as variant.
> >
> > In some cases, the variant will change the setup, such that expections
> > also change on teardown. Also pass variant to FIXTURE_TEARDOWN.
> >
> > The new FIXTURE_TEARDOWN logic is identical to that in FIXTURE_SETUP,
> > right above.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> Thanks! This was on my TODO list. :)
>
> Acked-by: Kees Cook <keescook@chromium.org>

Is this patch staged to be merged as is? Should I resubmit it?

Same question for another slightly older (2020-11-23) kselftest patch:

    tools/testing: add kselftest shell helper library
    https://patchwork.kernel.org/project/linux-kselftest/patch/20201123162508.585279-1-willemdebruijn.kernel@gmail.com/

Thanks,

  Willem

Re: [PATCH] selftests/harness: pass variant to teardown

[Kees Cook]

Hi Shuah,

Any news on these? I'd like to see them.

-Kees

On Fri, Feb 05, 2021 at 10:11:52PM -0500, Willem de Bruijn wrote:
> On Fri, Dec 11, 2020 at 1:15 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Thu, Dec 10, 2020 at 06:10:10PM -0500, Willem de Bruijn wrote:
> > > From: Willem de Bruijn <willemb@google.com>
> > >
> > > FIXTURE_VARIANT data is passed to FIXTURE_SETUP and TEST_F as variant.
> > >
> > > In some cases, the variant will change the setup, such that expections
> > > also change on teardown. Also pass variant to FIXTURE_TEARDOWN.
> > >
> > > The new FIXTURE_TEARDOWN logic is identical to that in FIXTURE_SETUP,
> > > right above.
> > >
> > > Signed-off-by: Willem de Bruijn <willemb@google.com>
> >
> > Thanks! This was on my TODO list. :)
> >
> > Acked-by: Kees Cook <keescook@chromium.org>
> 
> Is this patch staged to be merged as is? Should I resubmit it?
> 
> Same question for another slightly older (2020-11-23) kselftest patch:
> 
>     tools/testing: add kselftest shell helper library
>     https://patchwork.kernel.org/project/linux-kselftest/patch/20201123162508.585279-1-willemdebruijn.kernel@gmail.com/
> 
> Thanks,
> 
>   Willem

-- 
Kees Cook