Add kselftest_harness.h

Hi,

This third patch series make the seccomp/test_harness.h more generally available [1] and update the kselftest documentation with the Sphinx format. It also improve the Makefile of seccomp tests to take into account any kselftest_harness.h update.

[1] https://lkml.kernel.org/r/CAGXu5j+8CVz8vL51DRYXqOY=xc3zuKFf=PTENe88XYHzFYidUQ@mail.gmail.com

Regards,

Mickaël Salaün (6):
selftests: Make test_harness.h more generally available
selftests: Cosmetic renames in kselftest_harness.h selftests/seccomp: Force rebuild according to dependencies Documentation/dev-tools: Add kselftest Documentation/dev-tools: Use reStructuredText markups for kselftest Documentation/dev-tools: Add kselftest_harness documentation

 Documentation/00-INDEX                             |   2 -
 Documentation/dev-tools/index.rst                  |   1 +
 .../{kselftest.txt => dev-tools/kselftest.rst}     | 125 +++++++---
 MAINTAINERS                                        |   1 +
 .../test_harness.h => kselftest_harness.h}         | 270 +++++++++++++++------
 tools/testing/selftests/seccomp/Makefile           |   2 +
 tools/testing/selftests/seccomp/seccomp_bpf.c      |   2 +-
 7 files changed, 296 insertions(+), 107 deletions(-)
 rename Documentation/{kselftest.txt => dev-tools/kselftest.rst} (50%)
 rename tools/testing/selftests/{seccomp/test_harness.h => kselftest_harness.h} (80%)

Quoted 6 lines · Mickaël Salaün

Hi,

This third patch series make the seccomp/test_harness.h more generally available [1] and update the kselftest documentation with the Sphinx format. It also improve the Makefile of seccomp tests to take into account any kselftest_harness.h update.

Thanks for the test harness as well as updating kselftest documentation to rst format. Awesome. I will merge these into linux-kselftest after the 4.12-rc1 comes out.

I will have to defer to Jon Corbet for Documentation related changes and patches. Jon! Could you please review and give me an Ack.

Quoted 14 lines

[1] https://lkml.kernel.org/r/CAGXu5j+8CVz8vL51DRYXqOY=xc3zuKFf=PTENe88XYHzFYidUQ@mail.gmail.com

Regards,

Mickaël Salaün (6):
selftests: Make test_harness.h more generally available
selftests: Cosmetic renames in kselftest_harness.h selftests/seccomp: Force rebuild according to dependencies Documentation/dev-tools: Add kselftest Documentation/dev-tools: Use reStructuredText markups for kselftest Documentation/dev-tools: Add kselftest_harness documentation

Quoted 11 lines

 Documentation/00-INDEX                             |   2 -
 Documentation/dev-tools/index.rst                  |   1 +
 .../{kselftest.txt => dev-tools/kselftest.rst}     | 125 +++++++---
 MAINTAINERS                                        |   1 +
 .../test_harness.h => kselftest_harness.h}         | 270 +++++++++++++++------
 tools/testing/selftests/seccomp/Makefile           |   2 +
 tools/testing/selftests/seccomp/seccomp_bpf.c      |   2 +-
 7 files changed, 296 insertions(+), 107 deletions(-)
 rename Documentation/{kselftest.txt => dev-tools/kselftest.rst} (50%)
 rename tools/testing/selftests/{seccomp/test_harness.h => kselftest_harness.h} (80%)

thanks,
-- Shuah

Quoted 2 lines

I will have to defer to Jon Corbet for Documentation related changes and patches. Jon! Could you please review and give me an Ack.

Jonathan, what do you think about this patches?

Add metadata to kselftest_harness.h to be able to include the comments in the Sphinx documentation.

Changes since v2:
* add reference to the full documentation in the header file (suggested by Kees Cook)

Signed-off-by Mickaël Salaün <[email protected]>

Acked-by Kees Cook <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Shuah Khan <[email protected]>

Cc Will Drewry <[email protected]>

---
 Documentation/dev-tools/kselftest.rst       |  58 +++++++
 tools/testing/selftests/kselftest_harness.h | 259 ++++++++++++++++++++--------
 2 files changed, 245 insertions(+), 72 deletions(-)

Documentation/dev-tools/kselftest.rst +0 -0

index 9232ce94612c..92fc7cc3094b 100644

Documentation/dev-tools/kselftest.rst +27 -0

+
+Example
+-------
+
+.. code-block:: c
+
+    #include "../kselftest_harness.h"
+
+    TEST(standalone_test) {
+      do_some_stuff;
+      EXPECT_GT(10, stuff) {
+         stuff_state_t state;
+         enumerate_stuff_state(&state);
+         TH_LOG("expectation failed with state: %s", state.msg);
+      }
+      more_stuff;
+      ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
+      last_stuff;
+      EXPECT_EQ(0, last_stuff);
+    }
+
+    FIXTURE(my_fixture) {
+      mytype_t *data;
+      int awesomeness_level;
+    };
+    FIXTURE_SETUP(my_fixture) {
+      self->data = mytype_new();

Documentation/dev-tools/kselftest.rst +13 -0

+
+Helpers
+-------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: helpers
+
+
+Operators
+---------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: operators

tools/testing/selftests/kselftest_harness.h +1 -32

index 8ba227db46aa..b55be9807af4 100644
@@ -4,37 +4,7 @@
  *
  * kselftest_harness.h: simple C unit test helper.
  *
- * Usage:
- *   #include "../kselftest_harness.h"
- *   TEST(standalone_test) {
- *     do_some_stuff;
- *     EXPECT_GT(10, stuff) {
- *        stuff_state_t state;
- *        enumerate_stuff_state(&state);
- *        TH_LOG("expectation failed with state: %s", state.msg);
- *     }
- *     more_stuff;
- *     ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
- *     last_stuff;
- *     EXPECT_EQ(0, last_stuff);
- *   }
- *
- *   FIXTURE(my_fixture) {
- *     mytype_t *data;
- *     int awesomeness_level;
- *   };
- *   FIXTURE_SETUP(my_fixture) {
- *     self->data = mytype_new();
- *     ASSERT_NE(NULL, self->data);
- *   }
- *   FIXTURE_TEARDOWN(my_fixture) {
- *     mytype_free(self->data);
- *   }
- *   TEST_F(my_fixture, data_is_good) {
- *     EXPECT_EQ(1, is_my_data_good(self->data));
- *   }
- *
- *   TEST_HARNESS_MAIN
+ * See documentation in Documentation/dev-tools/kselftest.rst
  *
  * API inspired by code.google.com/p/googletest
  */

@@ -58,7 +28,13 @@
  * Exported APIs
  */
 
-/* TEST(name) { implementation }

Quoted 4 lines · On Tue, 16 May 2017 22:12:39 +0200 Mickaël Salaün <[email protected]> wrote:

Quoted 2 lines

I will have to defer to Jon Corbet for Documentation related changes and patches. Jon! Could you please review and give me an Ack.

Jonathan, what do you think about this patches?

Sorry, I missed that completely, looking now...

Quoted 28 lines

Add metadata to kselftest_harness.h to be able to include the comments in the Sphinx documentation.

Changes since v2:
* add reference to the full documentation in the header file (suggested by Kees Cook)

Signed-off-by Mickaël Salaün <[email protected]>

Acked-by Kees Cook <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Jonathan Corbet <[email protected]>

Cc Shuah Khan <[email protected]>

Cc Will Drewry <[email protected]>

---
 Documentation/dev-tools/kselftest.rst       |  58 +++++++
 tools/testing/selftests/kselftest_harness.h | 259 ++++++++++++++++++++--------
 2 files changed, 245 insertions(+), 72 deletions(-)

Documentation/dev-tools/kselftest.rst +2 -0

index 9232ce94612c..92fc7cc3094b 100644
@@ -120,3 +120,61 @@ Contributing new tests (details)
    executable which is not tested by default.
    TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
    test.
+
+Test Harness

Documentation/dev-tools/kselftest.rst Contributing new tests (details) · line 129

Quoted 5 lines

Documentation/dev-tools/kselftest.rst +5 -0

+============
+
+The *kselftest_harness.h* file contains useful helpers to build tests. The
+tests from *tools/testing/selftests/seccomp/seccomp_bpf.c* can be used as
+examples.

Minor quibble: in the spirit of minimizing markup, I'd probably not mark up the file names in this way.

Quoted 34 lines

Documentation/dev-tools/kselftest.rst +33 -0

+
+Example
+-------
+
+.. code-block:: c
+
+    #include "../kselftest_harness.h"
+
+    TEST(standalone_test) {
+      do_some_stuff;
+      EXPECT_GT(10, stuff) {
+         stuff_state_t state;
+         enumerate_stuff_state(&state);
+         TH_LOG("expectation failed with state: %s", state.msg);
+      }
+      more_stuff;
+      ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
+      last_stuff;
+      EXPECT_EQ(0, last_stuff);
+    }
+
+    FIXTURE(my_fixture) {
+      mytype_t *data;
+      int awesomeness_level;
+    };
+    FIXTURE_SETUP(my_fixture) {
+      self->data = mytype_new();
+      ASSERT_NE(NULL, self->data);
+    }
+    FIXTURE_TEARDOWN(my_fixture) {
+      mytype_free(self->data);
+    }
+    TEST_F(my_fixture, data_is_good) {

Documentation/dev-tools/kselftest.rst Contributing new tests (details) · line 166

Quoted 4 lines

Documentation/dev-tools/kselftest.rst +4 -0

+      EXPECT_EQ(1, is_my_data_good(self->data));
+    }
+
+    TEST_HARNESS_MAIN

So this was moved from the .h file. That's fine if you want to do it that way, but you could have also left it in place and included it with a :doc:
directive. Up to you.

Quoted 61 lines

Documentation/dev-tools/kselftest.rst +13 -0

+
+Helpers
+-------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: helpers
+
+
+Operators
+---------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: operators

tools/testing/selftests/kselftest_harness.h +1 -31

index 8ba227db46aa..b55be9807af4 100644
@@ -4,37 +4,7 @@
  *
  * kselftest_harness.h: simple C unit test helper.
  *
- * Usage:
- *   #include "../kselftest_harness.h"
- *   TEST(standalone_test) {
- *     do_some_stuff;
- *     EXPECT_GT(10, stuff) {
- *        stuff_state_t state;
- *        enumerate_stuff_state(&state);
- *        TH_LOG("expectation failed with state: %s", state.msg);
- *     }
- *     more_stuff;
- *     ASSERT_NE(some_stuff, NULL) TH_LOG("how did it happen?!");
- *     last_stuff;
- *     EXPECT_EQ(0, last_stuff);
- *   }
- *
- *   FIXTURE(my_fixture) {
- *     mytype_t *data;
- *     int awesomeness_level;
- *   };
- *   FIXTURE_SETUP(my_fixture) {
- *     self->data = mytype_new();
- *     ASSERT_NE(NULL, self->data);
- *   }
- *   FIXTURE_TEARDOWN(my_fixture) {
- *     mytype_free(self->data);
- *   }
- *   TEST_F(my_fixture, data_is_good) {
- *     EXPECT_EQ(1, is_my_data_good(self->data));
- *   }
- *
- *   TEST_HARNESS_MAIN
+ * See documentation in Documentation/dev-tools/kselftest.rst
  *
  * API inspired by code.google.com/p/googletest
  */

@@ -58,7 +28,13 @@
  * Exported APIs
  */
 

tools/testing/selftests/kselftest_harness.h line 40

Quoted 11 lines

tools/testing/selftests/kselftest_harness.h +7 -1

-/* TEST(name) { implementation }
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     TEST(name) { implementation }
+ *
  * Defines a test by name.
  * Names must be unique and tests must not be run in parallel.  The
  * implementation containing block is a function and scoping should be treated

It would be nicer to document these as actual functions, rather than using
DOC: blocks. It gives you all the standard formatting, index entries, cross-references, etc. A normal kerneldoc header will work with a macro like this.

I guess that's my most substantive comment. If you really want to do it this way instead I'll not raise a big fuss, but I would be curious to know what the reason is?

Thanks,

jon