[PATCH 0/2] test: fix naming of test functions in the log test suite

[PATCH 0/2] test: fix naming of test functions in the log test suite

[Heinrich Schuchardt]

Some Python subtests executed via the 'ut' console command were not
executed due to not following a required naming convention.

Add a description of the naming convention.
Fix the tests.

Heinrich Schuchardt (2):
  test: describe naming conventions for macro UNIT_TEST
  test: fix naming of test functions in the log test suite

 include/test/test.h      | 24 +++++++++++++++++++-
 test/log/nolog_test.c    | 24 ++++++++++----------
 test/log/syslog_test.c   | 48 ++++++++++++++++++++--------------------
 test/py/tests/test_ut.py | 17 +++++++++++++-
 4 files changed, 75 insertions(+), 38 deletions(-)

--
2.26.2

[PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

[Heinrich Schuchardt]

Strict naming conventions have to be followed for Python function
generate_ut_subtest() to collect C unit tests to be executed via
command 'ut'.

Describe the requirements both on the C as well on the Python side.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 include/test/test.h      | 24 +++++++++++++++++++++++-
 test/py/tests/test_ut.py | 17 ++++++++++++++++-
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/include/test/test.h b/include/test/test.h
index 2a75211008..029288de88 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -41,7 +41,29 @@ struct unit_test {
 	int flags;
 };

-/* Declare a new unit test */
+/**
+ * UNIT_TEST() - create linker generated list entry for unit a unit test
+ *
+ * The macro UNIT_TEST() is used to create a linker generated list entry. These
+ * list entries are enumerate tests that can be execute using the ut command.
+ * The list entries are used both by the implementation of the ut command as
+ * well as in a related Python test.
+ *
+ * For Python testing the subtests are collected in Python function
+ * generate_ut_subtest() by applying a regular expression to the lines of file
+ * u-boot.sym. The list entries have to follow strict naming conventions to be
+ * matched by the expression.
+ *
+ * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test suite
+ * foo that can be executed via command 'ut foo bar' and is implemented in
+ * function foo_test_bar().
+ *
+ * @_name:	concatenation of name of the test suite, "_test_", and the name
+ *		of the test
+ * @_flags:	an integer field that can be evaluated by the test suite
+ *		implementation
+ * @_suite:	name of the test suite concatenated with "_test"
+ */
 #define UNIT_TEST(_name, _flags, _suite)				\
 	ll_entry_declare(struct unit_test, _name, _suite) = {		\
 		.file = __FILE__,					\
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 6c7b8dd2b3..01c2b3ffa1 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -22,7 +22,22 @@ def test_ut_dm_init(u_boot_console):
             fh.write(data)

 def test_ut(u_boot_console, ut_subtest):
-    """Execute a "ut" subtest."""
+    """Execute a "ut" subtest.
+
+    The subtests are collected in function generate_ut_subtest() from linker
+    generated lists by applying a regular expression to the lines of file
+    u-boot.sym. The list entries are created using the C macro UNIT_TEST().
+
+    Strict naming conventions have to be followed to match the regular
+    expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in
+    test suite foo that can be executed via command 'ut foo bar' and is
+    implemented in C function foo_test_bar().
+
+    Args:
+        u_boot_console (ConsoleBase): U-Boot console
+        ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to
+            execute command 'ut foo bar'
+    """

     output = u_boot_console.run_command('ut ' + ut_subtest)
     assert output.endswith('Failures: 0')
--
2.26.2

[PATCH 2/2] test: fix naming of test functions in the log test suite

[Heinrich Schuchardt]

Both the nolog as well as the syslog tests were not found by Python
function generate_ut_subtest() due to not following the nameing
requirements imposed by the regular expression used to find linker
generated list entries in file u-boot.sym.

Adjust the naming of test functions.

With the patch the following tests are executed successfully for
sandbox_defconfig:

test/py/tests/test_ut.py::test_ut[ut_log_syslog_debug] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_err] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_info] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_nodebug] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_notice] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_warning] PASSED

The nolog tests are only executed if CONFIG_LOG=n and
CONFIG_CONSOLE_RECORD=y.

Reported-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 test/log/nolog_test.c  | 24 ++++++++++-----------
 test/log/syslog_test.c | 48 +++++++++++++++++++++---------------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/test/log/nolog_test.c b/test/log/nolog_test.c
index 84619521c9..c418ed07c9 100644
--- a/test/log/nolog_test.c
+++ b/test/log/nolog_test.c
@@ -19,7 +19,7 @@ DECLARE_GLOBAL_DATA_PTR;

 #define BUFFSIZE 32

-static int nolog_test_log_err(struct unit_test_state *uts)
+static int log_test_nolog_err(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -31,9 +31,9 @@ static int nolog_test_log_err(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_err);
+LOG_TEST(log_test_nolog_err);

-static int nolog_test_log_warning(struct unit_test_state *uts)
+static int log_test_nolog_warning(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -45,9 +45,9 @@ static int nolog_test_log_warning(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_warning);
+LOG_TEST(log_test_nolog_warning);

-static int nolog_test_log_notice(struct unit_test_state *uts)
+static int log_test_nolog_notice(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -59,9 +59,9 @@ static int nolog_test_log_notice(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_notice);
+LOG_TEST(log_test_nolog_notice);

-static int nolog_test_log_info(struct unit_test_state *uts)
+static int log_test_nolog_info(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -73,7 +73,7 @@ static int nolog_test_log_info(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_info);
+LOG_TEST(log_test_nolog_info);

 #undef _DEBUG
 #define _DEBUG 0
@@ -90,7 +90,7 @@ static int nolog_test_nodebug(struct unit_test_state *uts)
 }
 LOG_TEST(nolog_test_nodebug);

-static int nolog_test_log_nodebug(struct unit_test_state *uts)
+static int log_test_nolog_nodebug(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -102,7 +102,7 @@ static int nolog_test_log_nodebug(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_nodebug);
+LOG_TEST(log_test_nolog_nodebug);

 #undef _DEBUG
 #define _DEBUG 1
@@ -120,7 +120,7 @@ static int nolog_test_debug(struct unit_test_state *uts)
 }
 LOG_TEST(nolog_test_debug);

-static int nolog_test_log_debug(struct unit_test_state *uts)
+static int log_test_nolog_debug(struct unit_test_state *uts)
 {
 	char buf[BUFFSIZE];

@@ -132,4 +132,4 @@ static int nolog_test_log_debug(struct unit_test_state *uts)
 	ut_assertok(ut_check_console_end(uts));
 	return 0;
 }
-LOG_TEST(nolog_test_log_debug);
+LOG_TEST(log_test_nolog_debug);
diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
index 6ca5760eac..26536ebca7 100644
--- a/test/log/syslog_test.c
+++ b/test/log/syslog_test.c
@@ -92,12 +92,12 @@ static int sb_log_tx_handler(struct udevice *dev, void *packet,
 }

 /**
- * syslog_test_log_err() - test log_err() function
+ * log_test_syslog_err() - test log_err() function
  *
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_err(struct unit_test_state *uts)
+static int log_test_syslog_err(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -106,7 +106,7 @@ static int syslog_test_log_err(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<3>sandbox uboot: syslog_test_log_err() "
+	env.expected = "<3>sandbox uboot: log_test_syslog_err() "
 		       "testing log_err\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -119,15 +119,15 @@ static int syslog_test_log_err(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_err);
+LOG_TEST(log_test_syslog_err);

 /**
- * syslog_test_log_warning() - test log_warning() function
+ * log_test_syslog_warning() - test log_warning() function
  *
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_warning(struct unit_test_state *uts)
+static int log_test_syslog_warning(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -136,7 +136,7 @@ static int syslog_test_log_warning(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<4>sandbox uboot: syslog_test_log_warning() "
+	env.expected = "<4>sandbox uboot: log_test_syslog_warning() "
 		       "testing log_warning\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -150,15 +150,15 @@ static int syslog_test_log_warning(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_warning);
+LOG_TEST(log_test_syslog_warning);

 /**
- * syslog_test_log_notice() - test log_notice() function
+ * log_test_syslog_notice() - test log_notice() function
  *
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_notice(struct unit_test_state *uts)
+static int log_test_syslog_notice(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -167,7 +167,7 @@ static int syslog_test_log_notice(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<5>sandbox uboot: syslog_test_log_notice() "
+	env.expected = "<5>sandbox uboot: log_test_syslog_notice() "
 		       "testing log_notice\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -181,15 +181,15 @@ static int syslog_test_log_notice(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_notice);
+LOG_TEST(log_test_syslog_notice);

 /**
- * syslog_test_log_info() - test log_info() function
+ * log_test_syslog_info() - test log_info() function
  *
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_info(struct unit_test_state *uts)
+static int log_test_syslog_info(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -198,7 +198,7 @@ static int syslog_test_log_info(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<6>sandbox uboot: syslog_test_log_info() "
+	env.expected = "<6>sandbox uboot: log_test_syslog_info() "
 		       "testing log_info\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -212,15 +212,15 @@ static int syslog_test_log_info(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_info);
+LOG_TEST(log_test_syslog_info);

 /**
- * syslog_test_log_debug() - test log_debug() function
+ * log_test_syslog_debug() - test log_debug() function
  *
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_debug(struct unit_test_state *uts)
+static int log_test_syslog_debug(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -229,7 +229,7 @@ static int syslog_test_log_debug(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_DEBUG;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<7>sandbox uboot: syslog_test_log_debug() "
+	env.expected = "<7>sandbox uboot: log_test_syslog_debug() "
 		       "testing log_debug\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -243,10 +243,10 @@ static int syslog_test_log_debug(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_debug);
+LOG_TEST(log_test_syslog_debug);

 /**
- * syslog_test_log_nodebug() - test logging level filter
+ * log_test_syslog_nodebug() - test logging level filter
  *
  * Verify that log_debug() does not lead to a log message if the logging level
  * is set to LOGL_INFO.
@@ -254,7 +254,7 @@ LOG_TEST(syslog_test_log_debug);
  * @uts:	unit test state
  * Return:	0 = success
  */
-static int syslog_test_log_nodebug(struct unit_test_state *uts)
+static int log_test_syslog_nodebug(struct unit_test_state *uts)
 {
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
@@ -263,7 +263,7 @@ static int syslog_test_log_nodebug(struct unit_test_state *uts)
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
 	env_set("log_hostname", "sandbox");
-	env.expected = "<7>sandbox uboot: syslog_test_log_nodebug() "
+	env.expected = "<7>sandbox uboot: log_test_syslog_nodebug() "
 		       "testing log_debug\n";
 	env.uts = uts;
 	sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -277,4 +277,4 @@ static int syslog_test_log_nodebug(struct unit_test_state *uts)

 	return 0;
 }
-LOG_TEST(syslog_test_log_nodebug);
+LOG_TEST(log_test_syslog_nodebug);
--
2.26.2

[PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

[Stephen Warren]

On 5/6/20 10:26 AM, Heinrich Schuchardt wrote:
> Strict naming conventions have to be followed for Python function
> generate_ut_subtest() to collect C unit tests to be executed via
> command 'ut'.
> 
> Describe the requirements both on the C as well on the Python side.

> +/**
> + * UNIT_TEST() - create linker generated list entry for unit a unit test
> + *
> + * The macro UNIT_TEST() is used to create a linker generated list entry. These
> + * list entries are enumerate tests that can be execute using the ut command.
> + * The list entries are used both by the implementation of the ut command as
> + * well as in a related Python test.
> + *
> + * For Python testing the subtests are collected in Python function
> + * generate_ut_subtest() by applying a regular expression to the lines of file
> + * u-boot.sym. The list entries have to follow strict naming conventions to be
> + * matched by the expression.
> + *
> + * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test suite
> + * foo that can be executed via command 'ut foo bar' and is implemented in
> + * function foo_test_bar().
> + *
> + * @_name:	concatenation of name of the test suite, "_test_", and the name
> + *		of the test
> + * @_flags:	an integer field that can be evaluated by the test suite
> + *		implementation
> + * @_suite:	name of the test suite concatenated with "_test"
> + */

Perhaps the macro could simply take "foo" and "bar" as parameters, and
generate the function name foo_test_bar internally rather than having
the user pass it in. That way, compilation will actively fail if the
function isn't named correctly, since it won't match the reference
created by this macro.

To help make this easier, we could add another macro e.g.
UNIT_TEST_FUNC() that evaluates to just the expected function name, so
that people wouldn't have to know the naming convention when they
implement the function; they'd just write e.g.:

static int UNIT_TEST_FUNC(log, nolog_err)(struct unit_test_state *uts)

But certainly this series is a good first step, and fine even if we
don't implement this suggestion.

The series,
Reviewed-by: Stephen Warren <swarren@nvidia.com

[PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

[Simon Glass]

On Wed, 6 May 2020 at 17:28, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 5/6/20 10:26 AM, Heinrich Schuchardt wrote:
> > Strict naming conventions have to be followed for Python function
> > generate_ut_subtest() to collect C unit tests to be executed via
> > command 'ut'.
> >
> > Describe the requirements both on the C as well on the Python side.
>
> > +/**
> > + * UNIT_TEST() - create linker generated list entry for unit a unit test
> > + *
> > + * The macro UNIT_TEST() is used to create a linker generated list entry. These
> > + * list entries are enumerate tests that can be execute using the ut command.
> > + * The list entries are used both by the implementation of the ut command as
> > + * well as in a related Python test.
> > + *
> > + * For Python testing the subtests are collected in Python function
> > + * generate_ut_subtest() by applying a regular expression to the lines of file
> > + * u-boot.sym. The list entries have to follow strict naming conventions to be
> > + * matched by the expression.
> > + *
> > + * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test suite
> > + * foo that can be executed via command 'ut foo bar' and is implemented in
> > + * function foo_test_bar().
> > + *
> > + * @_name:   concatenation of name of the test suite, "_test_", and the name
> > + *           of the test
> > + * @_flags:  an integer field that can be evaluated by the test suite
> > + *           implementation
> > + * @_suite:  name of the test suite concatenated with "_test"
> > + */
>
> Perhaps the macro could simply take "foo" and "bar" as parameters, and
> generate the function name foo_test_bar internally rather than having
> the user pass it in. That way, compilation will actively fail if the
> function isn't named correctly, since it won't match the reference
> created by this macro.
>
> To help make this easier, we could add another macro e.g.
> UNIT_TEST_FUNC() that evaluates to just the expected function name, so
> that people wouldn't have to know the naming convention when they
> implement the function; they'd just write e.g.:
>
> static int UNIT_TEST_FUNC(log, nolog_err)(struct unit_test_state *uts)

I am not a huge fan of that. It looks weird to have the function name
auto-generated, and it defeats ctags, code search, etc.. Another
option might be to check for exported test/ functions that don't match
and print a warning?

>
> But certainly this series is a good first step, and fine even if we
> don't implement this suggestion.

Yes definitely.

>
> The series,
> Reviewed-by: Stephen Warren <swarren at nvidia.com

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

[Tom Rini]

On Wed, May 06, 2020 at 06:26:07PM +0200, Heinrich Schuchardt wrote:

> Strict naming conventions have to be followed for Python function
> generate_ut_subtest() to collect C unit tests to be executed via
> command 'ut'.
> 
> Describe the requirements both on the C as well on the Python side.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Stephen Warren <swarren@nvidia.com
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200508/783ffa64/attachment.sig>

[PATCH 2/2] test: fix naming of test functions in the log test suite

[Tom Rini]

On Wed, May 06, 2020 at 06:26:08PM +0200, Heinrich Schuchardt wrote:

> Both the nolog as well as the syslog tests were not found by Python
> function generate_ut_subtest() due to not following the nameing
> requirements imposed by the regular expression used to find linker
> generated list entries in file u-boot.sym.
> 
> Adjust the naming of test functions.
> 
> With the patch the following tests are executed successfully for
> sandbox_defconfig:
> 
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_debug] PASSED
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_err] PASSED
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_info] PASSED
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_nodebug] PASSED
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_notice] PASSED
> test/py/tests/test_ut.py::test_ut[ut_log_syslog_warning] PASSED
> 
> The nolog tests are only executed if CONFIG_LOG=n and
> CONFIG_CONSOLE_RECORD=y.
> 
> Reported-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200508/b8a14b92/attachment.sig>