[PATCH] docs/kselftest: add more guidelines for adding new tests

[PATCH] docs/kselftest: add more guidelines for adding new tests

[Muhammad Usama Anjum]

Add the following new guidelines:
- Add instruction to use lib.mk
- Add instruction about how to use headers from kernel source
- Add instruction to add .gitignore file
- Add instruction about how to add new test in selftests/Makefile
- Add instruction about different build commands to test

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Following patch is fixing build of kselftest when separate output
direcotry is specified using kernel's top most Makefile. It should be
accepted first:
https://lore.kernel.org/lkml/20220223191016.1658728-1-usama.anjum@collabora.com/
---
 Documentation/dev-tools/kselftest.rst | 46 ++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index a833ecf12fbc1..637f83d1450dc 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -208,6 +208,13 @@ In general, the rules for selftests are
 Contributing new tests (details)
 ================================
 
+ * Use lib.mk instead of writing Makefile from sratch. Specify flags and
+   binaries generation flags on need basis before including lib.mk. ::
+
+    CFLAGS = $(KHDR_INCLUDES)
+    TEST_GEN_PROGS := close_range_test
+    include ../lib.mk
+
  * Use TEST_GEN_XXX if such binaries or files are generated during
    compiling.
 
@@ -230,13 +237,50 @@ Contributing new tests (details)
  * First use the headers inside the kernel source and/or git repo, and then the
    system headers.  Headers for the kernel release as opposed to headers
    installed by the distro on the system should be the primary focus to be able
-   to find regressions.
+   to find regressions. Use KHDR_INCLUDES in Makefile to include headers from
+   the kernel source.
 
  * If a test needs specific kernel config options enabled, add a config file in
    the test directory to enable them.
 
    e.g: tools/testing/selftests/android/config
 
+ * Create a .gitignore file inside test directory and add all generated objects
+   in it.
+
+ * Add new test name in TARGETS in selftests/Makefile::
+
+    TARGETS += android
+
+ * All of the following build commands should be successful
+
+   - Same directory build of kselftests::
+
+      make kselftest-all
+      make kselftest-install
+      make kselftest-clean
+      make kselftest-gen_tar
+
+   - Build with absolute output directory path::
+
+      make kselftest-all O=/abs_build_path
+      make kselftest-install O=/abs_build_path
+      make kselftest-clean O=/abs_build_path
+      make kselftest-gen_tar O=/abs_build_path
+
+   - Build with relative output directory path::
+
+      make kselftest-all O=relative_path
+      make kselftest-install O=relative_path
+      make kselftest-clean O=relative_path
+      make kselftest-gen_tar O=relative_path
+
+   - Build from Makefile of selftests directly::
+
+      make -C tools/testing/selftests
+      make -C tools/testing/selftests O=/abs_build_path
+      make -C tools/testing/selftests O=relative_path
+
 Test Module
 ===========
 
-- 
2.30.2

Re: [PATCH] docs/kselftest: add more guidelines for adding new tests

[Bagas Sanjaya]

On 18/03/22 00.27, Muhammad Usama Anjum wrote:
> Add the following new guidelines:
> - Add instruction to use lib.mk
> - Add instruction about how to use headers from kernel source
> - Add instruction to add .gitignore file
> - Add instruction about how to add new test in selftests/Makefile
> - Add instruction about different build commands to test
> 

Too verbose, because people can figure out what were added in the diff
without explicitly mention them.
  
> + * Use lib.mk instead of writing Makefile from sratch. Specify flags and
> +   binaries generation flags on need basis before including lib.mk. ::
> +
> +    CFLAGS = $(KHDR_INCLUDES)
> +    TEST_GEN_PROGS := close_range_test
> +    include ../lib.mk
> +

I think what you mean is "In your Makefile, use facilities from lib.mk by
including it instead of reinventing the wheel.", right?

> + * Add new test name in TARGETS in selftests/Makefile::
> +
> +    TARGETS += android
> +
> + * All of the following build commands should be successful
> +
> +   - Same directory build of kselftests::
> +
> +      make kselftest-all
> +      make kselftest-install
> +      make kselftest-clean
> +      make kselftest-gen_tar
> +
> +   - Build with absolute output directory path::
> +
> +      make kselftest-all O=/abs_build_path
> +      make kselftest-install O=/abs_build_path
> +      make kselftest-clean O=/abs_build_path
> +      make kselftest-gen_tar O=/abs_build_path
> +
> +   - Build with relative output directory path::
> +
> +      make kselftest-all O=relative_path
> +      make kselftest-install O=relative_path
> +      make kselftest-clean O=relative_path
> +      make kselftest-gen_tar O=relative_path
> +
> +   - Build from Makefile of selftests directly::
> +
> +      make -C tools/testing/selftests
> +      make -C tools/testing/selftests O=/abs_build_path
> +      make -C tools/testing/selftests O=relative_path
> +

For simplicity, we can say "All changes should pass
kselftest-{all,install,clean,gen_tar} builds."

You don't need to spell out full command-line in the guideline unless
absolutely necessary, in general.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH] docs/kselftest: add more guidelines for adding new tests

[Muhammad Usama Anjum]

On 3/18/22 9:50 AM, Bagas Sanjaya wrote:
>> + * Use lib.mk instead of writing Makefile from sratch. Specify flags and
>> +   binaries generation flags on need basis before including lib.mk. ::
>> +
>> +    CFLAGS = $(KHDR_INCLUDES)
>> +    TEST_GEN_PROGS := close_range_test
>> +    include ../lib.mk
>> +
> 
> I think what you mean is "In your Makefile, use facilities from lib.mk by
> including it instead of reinventing the wheel.", right?
Yes, right.

> 
>> + * Add new test name in TARGETS in selftests/Makefile::
>> +
>> +    TARGETS += android
>> +
>> + * All of the following build commands should be successful
>> +
>> +   - Same directory build of kselftests::
>> +
>> +      make kselftest-all
>> +      make kselftest-install
>> +      make kselftest-clean
>> +      make kselftest-gen_tar
>> +
>> +   - Build with absolute output directory path::
>> +
>> +      make kselftest-all O=/abs_build_path
>> +      make kselftest-install O=/abs_build_path
>> +      make kselftest-clean O=/abs_build_path
>> +      make kselftest-gen_tar O=/abs_build_path
>> +
>> +   - Build with relative output directory path::
>> +
>> +      make kselftest-all O=relative_path
>> +      make kselftest-install O=relative_path
>> +      make kselftest-clean O=relative_path
>> +      make kselftest-gen_tar O=relative_path
>> +
>> +   - Build from Makefile of selftests directly::
>> +
>> +      make -C tools/testing/selftests
>> +      make -C tools/testing/selftests O=/abs_build_path
>> +      make -C tools/testing/selftests O=relative_path
>> +
> 
> For simplicity, we can say "All changes should pass
> kselftest-{all,install,clean,gen_tar} builds."
We (me and maintainer) want a list of commands to run before patch
submission. I'll use the {...} short hand method and update.

> You don't need to spell out full command-line in the guideline unless
> absolutely necessary, in general.
> 

-- 
Muhammad Usama Anjum