diff mbox series

selftests/harness: pass variant to teardown

Message ID 20201210231010.420298-1-willemdebruijn.kernel@gmail.com (mailing list archive)
State New
Headers show
Series selftests/harness: pass variant to teardown | expand

Commit Message

Willem de Bruijn Dec. 10, 2020, 11:10 p.m. UTC
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(-)

Comments

Jakub Kicinski Dec. 11, 2020, 12:26 a.m. UTC | #1
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>
Kees Cook Dec. 11, 2020, 6:15 p.m. UTC | #2
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>
Willem de Bruijn Feb. 6, 2021, 3:11 a.m. UTC | #3
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 Feb. 10, 2021, 8:57 p.m. UTC | #4
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
diff mbox series

Patch

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 = { \