[PATCH v3 0/6] Add kselftest_harness.h

[PATCH v3 0/6] Add kselftest_harness.h

[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.

[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%)

-- 
2.11.0

[PATCH v3 1/6] selftests: Make test_harness.h more generally available

[Mickaël Salaün]

The seccomp/test_harness.h file contains useful helpers to build tests.
Moving it to the selftest directory should benefit to other test
components.

Keep seccomp maintainers for this file.

Changes since v1:
* rename to kselftest_harness.h (suggested by Shuah Khan)
* keep maintainers

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Will Drewry <wad@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Shuah Khan <shuah@kernel.org>
Link: https://lkml.kernel.org/r/CAGXu5j+8CVz8vL51DRYXqOY=xc3zuKFf=PTENe88XYHzFYidUQ@mail.gmail.com
---
 MAINTAINERS                                                             | 1 +
 tools/testing/selftests/{seccomp/test_harness.h => kselftest_harness.h} | 0
 tools/testing/selftests/seccomp/seccomp_bpf.c                           | 2 +-
 3 files changed, 2 insertions(+), 1 deletion(-)
 rename tools/testing/selftests/{seccomp/test_harness.h => kselftest_harness.h} (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index c265a5fe4848..a42b0c1ca437 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11254,6 +11254,7 @@ F:	kernel/seccomp.c
 F:	include/uapi/linux/seccomp.h
 F:	include/linux/seccomp.h
 F:	tools/testing/selftests/seccomp/*
+F:	tools/testing/selftests/kselftest_harness.h
 K:	\bsecure_computing
 K:	\bTIF_SECCOMP\b
 
diff --git a/tools/testing/selftests/seccomp/test_harness.h b/tools/testing/selftests/kselftest_harness.h
similarity index 100%
rename from tools/testing/selftests/seccomp/test_harness.h
rename to tools/testing/selftests/kselftest_harness.h
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 03f1fa495d74..7ba94efb24fd 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -37,7 +37,7 @@
 #include <unistd.h>
 #include <sys/syscall.h>
 
-#include "test_harness.h"
+#include "../kselftest_harness.h"
 
 #ifndef PR_SET_PTRACER
 # define PR_SET_PTRACER 0x59616d61
-- 
2.11.0

[PATCH v3 2/6] selftests: Cosmetic renames in kselftest_harness.h

[Mickaël Salaün]

Keep the content consistent with the new name.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
---
 tools/testing/selftests/kselftest_harness.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index a786c69c7584..8ba227db46aa 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -2,10 +2,10 @@
  * Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
  * Use of this source code is governed by the GPLv2 license.
  *
- * test_harness.h: simple C unit test helper.
+ * kselftest_harness.h: simple C unit test helper.
  *
  * Usage:
- *   #include "test_harness.h"
+ *   #include "../kselftest_harness.h"
  *   TEST(standalone_test) {
  *     do_some_stuff;
  *     EXPECT_GT(10, stuff) {
@@ -38,8 +38,9 @@
  *
  * API inspired by code.google.com/p/googletest
  */
-#ifndef TEST_HARNESS_H_
-#define TEST_HARNESS_H_
+
+#ifndef __KSELFTEST_HARNESS_H
+#define __KSELFTEST_HARNESS_H
 
 #define _GNU_SOURCE
 #include <stdint.h>
@@ -532,4 +533,4 @@ static void __attribute__((constructor)) __constructor_order_first(void)
 		__constructor_order = _CONSTRUCTOR_ORDER_FORWARD;
 }
 
-#endif  /* TEST_HARNESS_H_ */
+#endif  /* __KSELFTEST_HARNESS_H *
-- 
2.11.0

[PATCH v3 3/6] selftests/seccomp: Force rebuild according to dependencies

[Mickaël Salaün]

Rebuild the seccomp tests when kselftest_harness.h is updated.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
---
 tools/testing/selftests/seccomp/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile
index 5fa6fd2246b1..aeb0c805f3ca 100644
--- a/tools/testing/selftests/seccomp/Makefile
+++ b/tools/testing/selftests/seccomp/Makefile
@@ -4,3 +4,5 @@ LDFLAGS += -lpthread
 
 include ../lib.mk
 
+$(TEST_GEN_PROGS): seccomp_bpf.c ../kselftest_harness.h
+	$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
-- 
2.11.0

[PATCH v3 4/6] Documentation/dev-tools: Add kselftest

[Mickaël Salaün]

Move kselftest.txt to dev-tools/kselftest.rst .

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Shuah Khan <shuah@kernel.org>
---
 Documentation/00-INDEX                                   | 2 --
 Documentation/{kselftest.txt => dev-tools/kselftest.rst} | 0
 2 files changed, 2 deletions(-)
 rename Documentation/{kselftest.txt => dev-tools/kselftest.rst} (100%)

diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX
index 793acf999e9e..924471664b5a 100644
--- a/Documentation/00-INDEX
+++ b/Documentation/00-INDEX
@@ -246,8 +246,6 @@ kprobes.txt
 	- documents the kernel probes debugging feature.
 kref.txt
 	- docs on adding reference counters (krefs) to kernel objects.
-kselftest.txt
-	- small unittests for (some) individual codepaths in the kernel.
 laptops/
 	- directory with laptop related info and laptop driver documentation.
 ldm.txt
diff --git a/Documentation/kselftest.txt b/Documentation/dev-tools/kselftest.rst
similarity index 100%
rename from Documentation/kselftest.txt
rename to Documentation/dev-tools/kselftest.rst
-- 
2.11.0

[PATCH v3 5/6] Documentation/dev-tools: Use reStructuredText markups for kselftest

[Mickaël Salaün]

Include and convert kselftest to the Sphinx format.

Changes since v2:
* lighten the modifications (suggested by Kees Cook)

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Shuah Khan <shuah@kernel.org>
---
 Documentation/dev-tools/index.rst     |  1 +
 Documentation/dev-tools/kselftest.rst | 67 +++++++++++++++++++++--------------
 2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 07d881147ef3..e50054c6aeaa 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -23,6 +23,7 @@ whole; patches welcome!
    kmemleak
    kmemcheck
    gdb-kernel-debugging
+   kselftest
 
 
 .. only::  subproject and html
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 5bd590335839..9232ce94612c 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -1,4 +1,6 @@
+======================
 Linux Kernel Selftests
+======================
 
 The kernel contains a set of "self tests" under the tools/testing/selftests/
 directory. These are intended to be small tests to exercise individual code
@@ -15,29 +17,34 @@ hotplug test is run on 2% of hotplug capable memory instead of 10%.
 Running the selftests (hotplug tests are run in limited mode)
 =============================================================
 
-To build the tests:
-  $ make -C tools/testing/selftests
+To build the tests::
+
+    make -C tools/testing/selftests
+
+To run the tests::
 
+    make -C tools/testing/selftests run_tests
 
-To run the tests:
-  $ make -C tools/testing/selftests run_tests
+To build and run the tests with a single command, use::
 
-To build and run the tests with a single command, use:
-  $ make kselftest
+    make kselftest
 
-- note that some tests will require root privileges.
+Note that some tests will require root privileges.
 
 
 Running a subset of selftests
-========================================
+=============================
+
 You can use the "TARGETS" variable on the make command line to specify
 single test to run, or a list of tests to run.
 
-To run only tests targeted for a single subsystem:
-  $  make -C tools/testing/selftests TARGETS=ptrace run_tests
+To run only tests targeted for a single subsystem::
+
+    make -C tools/testing/selftests TARGETS=ptrace run_tests
 
-You can specify multiple tests to build and run:
-  $  make TARGETS="size timers" kselftest
+You can specify multiple tests to build and run::
+
+    make TARGETS="size timers" kselftest
 
 See the top-level tools/testing/selftests/Makefile for the list of all
 possible targets.
@@ -46,13 +53,15 @@ possible targets.
 Running the full range hotplug selftests
 ========================================
 
-To build the hotplug tests:
-  $ make -C tools/testing/selftests hotplug
+To build the hotplug tests::
+
+    make -C tools/testing/selftests hotplug
+
+To run the hotplug tests::
 
-To run the hotplug tests:
-  $ make -C tools/testing/selftests run_hotplug
+    make -C tools/testing/selftests run_hotplug
 
-- note that some tests will require root privileges.
+Note that some tests will require root privileges.
 
 
 Install selftests
@@ -62,13 +71,15 @@ You can use kselftest_install.sh tool installs selftests in default
 location which is tools/testing/selftests/kselftest or a user specified
 location.
 
-To install selftests in default location:
-   $ cd tools/testing/selftests
-   $ ./kselftest_install.sh
+To install selftests in default location::
 
-To install selftests in a user specified location:
-   $ cd tools/testing/selftests
-   $ ./kselftest_install.sh install_dir
+    cd tools/testing/selftests
+    ./kselftest_install.sh
+
+To install selftests in a user specified location::
+
+    cd tools/testing/selftests
+    ./kselftest_install.sh install_dir
 
 Running installed selftests
 ===========================
@@ -79,8 +90,10 @@ named "run_kselftest.sh" to run the tests.
 You can simply do the following to run the installed Kselftests. Please
 note some tests will require root privileges.
 
-cd kselftest
-./run_kselftest.sh
+::
+
+    cd kselftest
+    ./run_kselftest.sh
 
 Contributing new tests
 ======================
@@ -96,8 +109,8 @@ In general, the rules for selftests are
  * Don't cause the top-level "make run_tests" to fail if your feature is
    unconfigured.
 
-Contributing new tests(details)
-===============================
+Contributing new tests (details)
+================================
 
  * Use TEST_GEN_XXX if such binaries or files are generated during
    compiling.
-- 
2.11.0

[PATCH v3 6/6] Documentation/dev-tools: Add kselftest_harness documentation

[Mickaël Salaün]

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 <mic@digikod.net>
Acked-by: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
---
 Documentation/dev-tools/kselftest.rst       |  58 +++++++
 tools/testing/selftests/kselftest_harness.h | 259 ++++++++++++++++++++--------
 2 files changed, 245 insertions(+), 72 deletions(-)

diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 9232ce94612c..92fc7cc3094b 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -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
+============
+
+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.
+
+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) {
+      EXPECT_EQ(1, is_my_data_good(self->data));
+    }
+
+    TEST_HARNESS_MAIN
+
+
+Helpers
+-------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: helpers
+
+
+Operators
+---------
+
+.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
+    :doc: operators
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 8ba227db46aa..b55be9807af4 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -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 }
+/**
+ * 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
@@ -68,7 +44,13 @@
  */
 #define TEST TEST_API(TEST)
 
-/* TEST_SIGNAL(name, signal) { implementation }
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     TEST_SIGNAL(name, signal) { implementation }
+ *
  * Defines a test by name and the expected term signal.
  * Names must be unique and tests must not be run in parallel.  The
  * implementation containing block is a function and scoping should be treated
@@ -78,25 +60,43 @@
  */
 #define TEST_SIGNAL TEST_API(TEST_SIGNAL)
 
-/* FIXTURE(datatype name) {
- *   type property1;
- *   ...
- * };
- * Defines the data provided to TEST_F()-defined tests as |self|.  It should be
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     FIXTURE(datatype name) {
+ *       type property1;
+ *       ...
+ *     };
+ *
+ * Defines the data provided to TEST_F()-defined tests as \|self\|.  It should be
  * populated and cleaned up using FIXTURE_SETUP and FIXTURE_TEARDOWN.
  */
 #define FIXTURE TEST_API(FIXTURE)
 
-/* FIXTURE_DATA(datatype name)
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     FIXTURE_DATA(datatype name)
+ *
  * This call may be used when the type of the fixture data
  * is needed.  In general, this should not be needed unless
- * the |self| is being passed to a helper directly.
+ * the \|self\| is being passed to a helper directly.
  */
 #define FIXTURE_DATA TEST_API(FIXTURE_DATA)
 
-/* FIXTURE_SETUP(fixture name) { implementation }
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     FIXTURE_SETUP(fixture name) { implementation }
+ *
  * Populates the required "setup" function for a fixture.  An instance of the
- * datatype defined with _FIXTURE_DATA will be exposed as |self| for the
+ * datatype defined with _FIXTURE_DATA will be exposed as \|self\| for the
  * implementation.
  *
  * ASSERT_* are valid for use in this context and will prempt the execution
@@ -106,84 +106,199 @@
  */
 #define FIXTURE_SETUP TEST_API(FIXTURE_SETUP)
 
-/* FIXTURE_TEARDOWN(fixture name) { implementation }
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     FIXTURE_TEARDOWN(fixture name) { implementation }
+ *
  * Populates the required "teardown" function for a fixture.  An instance of the
- * datatype defined with _FIXTURE_DATA will be exposed as |self| for the
+ * datatype defined with _FIXTURE_DATA will be exposed as \|self\| for the
  * implementation to clean up.
  *
  * A bare "return;" statement may be used to return early.
  */
 #define FIXTURE_TEARDOWN TEST_API(FIXTURE_TEARDOWN)
 
-/* TEST_F(fixture, name) { implementation }
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     TEST_F(fixture, name) { implementation }
+ *
  * Defines a test that depends on a fixture (e.g., is part of a test case).
- * Very similar to TEST() except that |self| is the setup instance of fixture's
+ * Very similar to TEST() except that \|self\| is the setup instance of fixture's
  * datatype exposed for use by the implementation.
  */
 #define TEST_F TEST_API(TEST_F)
 
 #define TEST_F_SIGNAL TEST_API(TEST_F_SIGNAL)
 
-/* Use once to append a main() to the test file. E.g.,
- *   TEST_HARNESS_MAIN
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     TEST_HARNESS_MAIN
+ *
+ * Use once to append a main() to the test file.
  */
 #define TEST_HARNESS_MAIN TEST_API(TEST_HARNESS_MAIN)
 
-/*
+/**
+ * DOC: operators
+ *
  * Operators for use in TEST and TEST_F.
  * ASSERT_* calls will stop test execution immediately.
  * EXPECT_* calls will emit a failure warning, note it, and continue.
  */
 
-/* ASSERT_EQ(expected, measured): expected == measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_EQ(expected, measured): expected == measured
+ */
 #define ASSERT_EQ TEST_API(ASSERT_EQ)
-/* ASSERT_NE(expected, measured): expected != measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_NE(expected, measured): expected != measured
+ */
 #define ASSERT_NE TEST_API(ASSERT_NE)
-/* ASSERT_LT(expected, measured): expected < measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_LT(expected, measured): expected < measured
+ */
 #define ASSERT_LT TEST_API(ASSERT_LT)
-/* ASSERT_LE(expected, measured): expected <= measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_LE(expected, measured): expected <= measured
+ */
 #define ASSERT_LE TEST_API(ASSERT_LE)
-/* ASSERT_GT(expected, measured): expected > measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_GT(expected, measured): expected > measured
+ */
 #define ASSERT_GT TEST_API(ASSERT_GT)
-/* ASSERT_GE(expected, measured): expected >= measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_GE(expected, measured): expected >= measured
+ */
 #define ASSERT_GE TEST_API(ASSERT_GE)
-/* ASSERT_NULL(measured): NULL == measured */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_NULL(measured): NULL == measured
+ */
 #define ASSERT_NULL TEST_API(ASSERT_NULL)
-/* ASSERT_TRUE(measured): measured != 0 */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_TRUE(measured): measured != 0
+ */
 #define ASSERT_TRUE TEST_API(ASSERT_TRUE)
-/* ASSERT_FALSE(measured): measured == 0 */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_FALSE(measured): measured == 0
+ */
 #define ASSERT_FALSE TEST_API(ASSERT_FALSE)
-/* ASSERT_STREQ(expected, measured): !strcmp(expected, measured) */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_STREQ(expected, measured): !strcmp(expected, measured)
+ */
 #define ASSERT_STREQ TEST_API(ASSERT_STREQ)
-/* ASSERT_STRNE(expected, measured): strcmp(expected, measured) */
+/**
+ * DOC: operators
+ *
+ * * ASSERT_STRNE(expected, measured): strcmp(expected, measured)
+ */
 #define ASSERT_STRNE TEST_API(ASSERT_STRNE)
-/* EXPECT_EQ(expected, measured): expected == measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_EQ(expected, measured): expected == measured
+ */
 #define EXPECT_EQ TEST_API(EXPECT_EQ)
-/* EXPECT_NE(expected, measured): expected != measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_NE(expected, measured): expected != measured
+ */
 #define EXPECT_NE TEST_API(EXPECT_NE)
-/* EXPECT_LT(expected, measured): expected < measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_LT(expected, measured): expected < measured
+ */
 #define EXPECT_LT TEST_API(EXPECT_LT)
-/* EXPECT_LE(expected, measured): expected <= measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_LE(expected, measured): expected <= measured
+ */
 #define EXPECT_LE TEST_API(EXPECT_LE)
-/* EXPECT_GT(expected, measured): expected > measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_GT(expected, measured): expected > measured
+ */
 #define EXPECT_GT TEST_API(EXPECT_GT)
-/* EXPECT_GE(expected, measured): expected >= measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_GE(expected, measured): expected >= measured
+ */
 #define EXPECT_GE TEST_API(EXPECT_GE)
-/* EXPECT_NULL(measured): NULL == measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_NULL(measured): NULL == measured
+ */
 #define EXPECT_NULL TEST_API(EXPECT_NULL)
-/* EXPECT_TRUE(measured): 0 != measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_TRUE(measured): 0 != measured
+ */
 #define EXPECT_TRUE TEST_API(EXPECT_TRUE)
-/* EXPECT_FALSE(measured): 0 == measured */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_FALSE(measured): 0 == measured
+ */
 #define EXPECT_FALSE TEST_API(EXPECT_FALSE)
-/* EXPECT_STREQ(expected, measured): !strcmp(expected, measured) */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_STREQ(expected, measured): !strcmp(expected, measured)
+ */
 #define EXPECT_STREQ TEST_API(EXPECT_STREQ)
-/* EXPECT_STRNE(expected, measured): strcmp(expected, measured) */
+/**
+ * DOC: operators
+ *
+ * * EXPECT_STRNE(expected, measured): strcmp(expected, measured)
+ */
 #define EXPECT_STRNE TEST_API(EXPECT_STRNE)
 
-/* TH_LOG(format, ...)
+/**
+ * DOC: helpers
+ *
+ * .. code-block:: c
+ *
+ *     TH_LOG(format, ...)
+ *
  * Optional debug logging function available for use in tests.
  * Logging may be enabled or disabled by defining TH_LOG_ENABLED.
  * E.g., #define TH_LOG_ENABLED 1
+ *
  * If no definition is provided, logging is enabled by default.
  */
 #define TH_LOG  TEST_API(TH_LOG)
-- 
2.11.0

Re: [PATCH v3 0/6] Add kselftest_harness.h

[Shuah Khan]

On 05/03/2017 04:26 PM, Mickaël Salaün wrote:
> 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.

> 
> [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%)
> 

thanks,
-- Shuah

Re: [PATCH v3 0/6] Add kselftest_harness.h

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 2009 bytes --]


On 04/05/2017 15:58, Shuah Khan wrote:
> On 05/03/2017 04:26 PM, Mickaël Salaün wrote:
>> 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.

Jonathan, what do you think about this patches?

> 
>>
>> [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%)
>>
> 
> thanks,
> -- Shuah
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/6] Add kselftest_harness.h

[Jonathan Corbet]

On Tue, 16 May 2017 22:12:39 +0200
Mickaël Salaün <mic@digikod.net> wrote:

> > 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...

> 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 <mic@digikod.net>
> Acked-by: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Will Drewry <wad@chromium.org>
> ---
>  Documentation/dev-tools/kselftest.rst       |  58 +++++++
>  tools/testing/selftests/kselftest_harness.h | 259 ++++++++++++++++++++--------
>  2 files changed, 245 insertions(+), 72 deletions(-)
> 
> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> index 9232ce94612c..92fc7cc3094b 100644
> --- a/Documentation/dev-tools/kselftest.rst
> +++ b/Documentation/dev-tools/kselftest.rst
> @@ -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
> +============
> +
> +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.

> +
> +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) {
> +      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.

> +
> +Helpers
> +-------
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +    :doc: helpers
> +
> +
> +Operators
> +---------
> +
> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
> +    :doc: operators
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 8ba227db46aa..b55be9807af4 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -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 }
> +/**
> + * 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

Re: [PATCH v3 0/6] Add kselftest_harness.h

[Mickaël Salaün]

[-- Attachment #1.1: Type: text/plain, Size: 5982 bytes --]



On 16/05/2017 22:29, Jonathan Corbet wrote:
> On Tue, 16 May 2017 22:12:39 +0200
> Mickaël Salaün <mic@digikod.net> wrote:
> 
>>> 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...
> 
>> 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 <mic@digikod.net>
>> Acked-by: Kees Cook <keescook@chromium.org>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Shuah Khan <shuah@kernel.org>
>> Cc: Will Drewry <wad@chromium.org>
>> ---
>>  Documentation/dev-tools/kselftest.rst       |  58 +++++++
>>  tools/testing/selftests/kselftest_harness.h | 259 ++++++++++++++++++++--------
>>  2 files changed, 245 insertions(+), 72 deletions(-)
>>
>> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
>> index 9232ce94612c..92fc7cc3094b 100644
>> --- a/Documentation/dev-tools/kselftest.rst
>> +++ b/Documentation/dev-tools/kselftest.rst
>> @@ -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
>> +============
>> +
>> +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.

OK

> 
>> +
>> +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) {
>> +      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.

Keeping it in the .h file means not benefiting from the C syntax
highlighting (even with ".. code-block:: c"). It looks like a bug, though.

> 
>> +
>> +Helpers
>> +-------
>> +
>> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
>> +    :doc: helpers
>> +
>> +
>> +Operators
>> +---------
>> +
>> +.. kernel-doc:: tools/testing/selftests/kselftest_harness.h
>> +    :doc: operators
>> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
>> index 8ba227db46aa..b55be9807af4 100644
>> --- a/tools/testing/selftests/kselftest_harness.h
>> +++ b/tools/testing/selftests/kselftest_harness.h
>> @@ -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 }
>> +/**
>> + * 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 can do that but a macro defined as "#define TEST TEST_API(TEST)"
doesn't get an argument with kerneldoc. I guess it is better than a DOC
block, though.

> 
> 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
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]