From d3fdbd0afc3ee0ed6d34e72ff715fe31cf24976c Mon Sep 17 00:00:00 2001 From: Gregers Gram Rygg Date: Fri, 14 Jan 2022 13:56:02 +0100 Subject: [PATCH 1/2] scripts: unity: Fix function definition regex bug The whitespace fix to this regex last week triggered another bug in libmodem. They have a typedef that the regex parsed as a function definition because it accepted too many character in the argument list of a function. This fix updates the regex to only allow characters you would expect in an argument list of a function def. Characters expected in the argument list of a function def: \w A-Z a-z 0-9 and _ \s All whitespace , Argument separator * Pointers \. Variadic argument \[ Array start \] Array end Signed-off-by: Gregers Gram Rygg --- scripts/unity/func_name_list.py | 2 +- scripts/unity/header_prepare.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/unity/func_name_list.py b/scripts/unity/func_name_list.py index 1e9d8d4714d4..27718b9887bc 100755 --- a/scripts/unity/func_name_list.py +++ b/scripts/unity/func_name_list.py @@ -14,7 +14,7 @@ def func_names_from_header(in_file, out_file): with open(out_file, 'w') as f_out: # Regex match all function names in the header file - x = re.findall(r"^\s*(?:\w+[*\s]+)+(\w+?)\s*\([^\\]*?\)\s*;", + x = re.findall(r"^\s*(?:\w+[*\s]+)+(\w+?)\s*\([\w\s,*\.\[\]]*?\)\s*;", content, re.M | re.S) for item in x: f_out.write(item + "\n") diff --git a/scripts/unity/header_prepare.py b/scripts/unity/header_prepare.py index aa30257cb16c..901c3c06e6fe 100755 --- a/scripts/unity/header_prepare.py +++ b/scripts/unity/header_prepare.py @@ -63,7 +63,7 @@ def header_prepare(in_file, out_file, out_wrap_file): # Prepare file with functions prefixed with __wrap_ that will be used for # mock generation. func_pattern = re.compile( - r"^\s*((?:\w+[*\s]+)+)(\w+?\s*\([^\\{}#]*?\)\s*;)", re.M) + r"^\s*((?:\w+[*\s]+)+)(\w+?\s*\([\w\s,*\.\[\]]*?\)\s*;)", re.M) content2 = func_pattern.sub(r"\n\1__wrap_\2", content) with open(out_wrap_file, 'w') as f_wrap: From 12fc8eded9061eeedc66419a47c49072840cb345 Mon Sep 17 00:00:00 2001 From: Gregers Gram Rygg Date: Wed, 19 Jan 2022 09:12:43 +0100 Subject: [PATCH 2/2] tests: unity: Separate example test and wrap tests The python scripts to prefix functions with __wrap was tested using the example_test. This test is mean as an example of how to do unit testing. Moved the existing wrap tests and some new ones to a new test suite to avoid adding more irrelevant tests to the example test. Signed-off-by: Gregers Gram Rygg --- scripts/unity/func_name_list.py | 1 + scripts/unity/header_prepare.py | 1 + tests/unity/example_test/src/example_test.c | 3 - tests/unity/example_test/src/foo/foo.c | 5 + tests/unity/example_test/src/foo/foo.h | 39 +------- .../unity/example_test/src/foo/foo_internal.h | 11 --- tests/unity/example_test/src/uut/uut.c | 4 +- tests/unity/wrap_test/CMakeLists.txt | 29 ++++++ tests/unity/wrap_test/prj.conf | 6 ++ tests/unity/wrap_test/src/call/call.c | 45 +++++++++ tests/unity/wrap_test/src/call/call.h | 17 ++++ .../wrap_test/src/test_code/.clang-format | 3 + .../unity/wrap_test/src/test_code/test_code.c | 7 ++ .../unity/wrap_test/src/test_code/test_code.h | 89 ++++++++++++++++++ tests/unity/wrap_test/src/wrap_test.c | 91 +++++++++++++++++++ tests/unity/wrap_test/testcase.yaml | 5 + 16 files changed, 301 insertions(+), 55 deletions(-) delete mode 100644 tests/unity/example_test/src/foo/foo_internal.h create mode 100644 tests/unity/wrap_test/CMakeLists.txt create mode 100644 tests/unity/wrap_test/prj.conf create mode 100644 tests/unity/wrap_test/src/call/call.c create mode 100644 tests/unity/wrap_test/src/call/call.h create mode 100644 tests/unity/wrap_test/src/test_code/.clang-format create mode 100644 tests/unity/wrap_test/src/test_code/test_code.c create mode 100644 tests/unity/wrap_test/src/test_code/test_code.h create mode 100644 tests/unity/wrap_test/src/wrap_test.c create mode 100644 tests/unity/wrap_test/testcase.yaml diff --git a/scripts/unity/func_name_list.py b/scripts/unity/func_name_list.py index 27718b9887bc..bc3e9eb5a37c 100755 --- a/scripts/unity/func_name_list.py +++ b/scripts/unity/func_name_list.py @@ -14,6 +14,7 @@ def func_names_from_header(in_file, out_file): with open(out_file, 'w') as f_out: # Regex match all function names in the header file + # Tests for validating the regex in tests/unity/wrap x = re.findall(r"^\s*(?:\w+[*\s]+)+(\w+?)\s*\([\w\s,*\.\[\]]*?\)\s*;", content, re.M | re.S) for item in x: diff --git a/scripts/unity/header_prepare.py b/scripts/unity/header_prepare.py index 901c3c06e6fe..3411cf30b21a 100755 --- a/scripts/unity/header_prepare.py +++ b/scripts/unity/header_prepare.py @@ -7,6 +7,7 @@ import re import argparse +# Tests for validating the regexes are located tests/unity/wrap def header_prepare(in_file, out_file, out_wrap_file): with open(in_file) as f_in: diff --git a/tests/unity/example_test/src/example_test.c b/tests/unity/example_test/src/example_test.c index 16c093ff2290..a41308badf78 100644 --- a/tests/unity/example_test/src/example_test.c +++ b/tests/unity/example_test/src/example_test.c @@ -30,10 +30,7 @@ void test_uut_init(void) int err; __wrap_foo_init_ExpectAndReturn(NULL, 0); - __wrap_foo_syscall_Expect(); __wrap_foo_execute_ExpectAndReturn(0); - __wrap_foo_execute2_ExpectAndReturn(0); - __wrap_foo_execute3_ExpectAndReturn(0); err = uut_init(NULL); TEST_ASSERT_EQUAL(0, err); diff --git a/tests/unity/example_test/src/foo/foo.c b/tests/unity/example_test/src/foo/foo.c index e49b54acbce7..46e6b145b3c2 100644 --- a/tests/unity/example_test/src/foo/foo.c +++ b/tests/unity/example_test/src/foo/foo.c @@ -11,3 +11,8 @@ int foo_init(void *handle) /* This implementation will be wrapped and mocked. */ return 0; } + +int foo_execute(void) +{ + return 0; +} diff --git a/tests/unity/example_test/src/foo/foo.h b/tests/unity/example_test/src/foo/foo.h index 812bf59f7de0..96de2719427c 100644 --- a/tests/unity/example_test/src/foo/foo.h +++ b/tests/unity/example_test/src/foo/foo.h @@ -9,45 +9,8 @@ #include -#include "foo_internal.h" -// c++ comment -/* c comment */ -/* c - * multi line comment - */ -#define FOO_ADDR "https://my.page.com/somewhere" - int foo_init(void *handle); -static const char test_string[] = "test string"; - -#define TEST_MACRO(xx) do { \ - const STRUCT_SECTION_ITERABLE(settings_handler_static, \ - name) \ -} while (0) - -static inline int foo_execute(void) -{ - foo_t val = 0; - - return (int)val; -} - -static ALWAYS_INLINE int foo_execute2(void) -{ - return 0; -} - -inline static int foo_execute3(void) -{ - return 0; -} - -__syscall void foo_syscall(void); - -static inline void z_impl_foo_syscall(void) -{ - /* empty */ -} +int foo_execute(void); #endif /* __FOO_H */ diff --git a/tests/unity/example_test/src/foo/foo_internal.h b/tests/unity/example_test/src/foo/foo_internal.h deleted file mode 100644 index b4d9c56e1fc4..000000000000 --- a/tests/unity/example_test/src/foo/foo_internal.h +++ /dev/null @@ -1,11 +0,0 @@ -/* - * Copyright (c) 2020 Nordic Semiconductor ASA - * - * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause - */ -#ifndef __FOO_INTERNAL_H -#define __FOO_INTERNAL_H - -typedef int foo_t; - -#endif /* __FOO_INTERNAL_H */ diff --git a/tests/unity/example_test/src/uut/uut.c b/tests/unity/example_test/src/uut/uut.c index f92450302304..7b5a1bd20633 100644 --- a/tests/unity/example_test/src/uut/uut.c +++ b/tests/unity/example_test/src/uut/uut.c @@ -24,7 +24,5 @@ int uut_init(void *handle) return err; } - foo_syscall(); - - return foo_execute() + foo_execute2() + foo_execute3(); + return foo_execute(); } diff --git a/tests/unity/wrap_test/CMakeLists.txt b/tests/unity/wrap_test/CMakeLists.txt new file mode 100644 index 000000000000..a6b1dabb9f4e --- /dev/null +++ b/tests/unity/wrap_test/CMakeLists.txt @@ -0,0 +1,29 @@ +# +# Copyright (c) 2022 Nordic Semiconductor +# +# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause +# + +cmake_minimum_required(VERSION 3.20.0) + +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(wrap_test) + +# generate runner for the test +test_runner_generate(src/wrap_test.c) + +# create mocks for test_code functions +cmock_handle(src/test_code/test_code.h test_code) + +# add module test_code +target_sources(app PRIVATE src/test_code/test_code.c) +target_include_directories(app PRIVATE src) + +# add module call +target_sources(app PRIVATE src/call/call.c) +target_include_directories(app PRIVATE ./src/call) + +# add test file +target_sources(app PRIVATE src/wrap_test.c) +target_include_directories(app PRIVATE .) +target_include_directories(app PRIVATE src/test_code) diff --git a/tests/unity/wrap_test/prj.conf b/tests/unity/wrap_test/prj.conf new file mode 100644 index 000000000000..60a04c23b6ff --- /dev/null +++ b/tests/unity/wrap_test/prj.conf @@ -0,0 +1,6 @@ +# +# Copyright (c) 2022 Nordic Semiconductor ASA +# +# SPDX-License-Identifier: LicenseRef-Nordic-5-Clause +# +CONFIG_UNITY=y diff --git a/tests/unity/wrap_test/src/call/call.c b/tests/unity/wrap_test/src/call/call.c new file mode 100644 index 000000000000..43b34c5e5567 --- /dev/null +++ b/tests/unity/wrap_test/src/call/call.c @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ +#include +#include + +#include "call.h" +#include "test_code/test_code.h" + +void call_syscall(void) +{ + syscall(); +} + +int call_static_inline_fn(void) +{ + return static_inline_fn(); +} + +int call_inline_static_fn(void) +{ + return inline_static_fn(); +} + +int call_always_inline_fn(void) +{ + return always_inline_fn(); +} + +int call_after_macro(void) +{ + return after_macro(); +} + +int call_extra_whitespace(void) +{ + return extra_whitespace(); +} + +int call_multiline_def(int arg, int len) +{ + return multiline_def(arg, len); +} diff --git a/tests/unity/wrap_test/src/call/call.h b/tests/unity/wrap_test/src/call/call.h new file mode 100644 index 000000000000..cb74d508740f --- /dev/null +++ b/tests/unity/wrap_test/src/call/call.h @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ +#ifndef __CALL_H +#define __CALL_H + +void call_syscall(void); +int call_static_inline_fn(void); +int call_inline_static_fn(void); +int call_always_inline_fn(void); +int call_after_macro(void); +int call_extra_whitespace(void); +int call_multiline_def(int arg, int len); + +#endif /* __CALL_H */ diff --git a/tests/unity/wrap_test/src/test_code/.clang-format b/tests/unity/wrap_test/src/test_code/.clang-format new file mode 100644 index 000000000000..36f022e46efc --- /dev/null +++ b/tests/unity/wrap_test/src/test_code/.clang-format @@ -0,0 +1,3 @@ +{ + "DisableFormat": true +} diff --git a/tests/unity/wrap_test/src/test_code/test_code.c b/tests/unity/wrap_test/src/test_code/test_code.c new file mode 100644 index 000000000000..49e46b98e23f --- /dev/null +++ b/tests/unity/wrap_test/src/test_code/test_code.c @@ -0,0 +1,7 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ + +#include "test_code/test_code.h" diff --git a/tests/unity/wrap_test/src/test_code/test_code.h b/tests/unity/wrap_test/src/test_code/test_code.h new file mode 100644 index 000000000000..e3cc11819f41 --- /dev/null +++ b/tests/unity/wrap_test/src/test_code/test_code.h @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ + +/* + * DO NOT FORMAT THIS FILE! + * + * It contains various code blocks that is used to validate the test wrapper + * scripts (func_name_list.py and header_parse.py) which renames function + * declarations. The regular expression has had several bugs in the past, and + * the formating of the code here is used to validate that the fixes still work + * when modifying the regex. + * + * The code in this file ensure that the test wrapper scripts can handle the + * situations described in following list of issues: + * https://github.com/nrfconnect/sdk-nrf/pull/1413 + * https://github.com/nrfconnect/sdk-nrf/pull/2182 + * https://github.com/nrfconnect/sdk-nrf/pull/3432 + * https://github.com/nrfconnect/sdk-nrf/pull/5069 + * https://github.com/nrfconnect/sdk-nrf/pull/5814 + * https://github.com/nrfconnect/sdk-nrf/pull/6478 + * https://github.com/nrfconnect/sdk-nrf/pull/6547 + */ + +#ifndef __TEST_CODE_H +#define __TEST_CODE_H + +// c++ comment +/* c comment */ +/* c + * multi line comment + */ +#define SOME_ADDR "https://my.page.com/somewhere" + +static const char test_string[] = "test string"; + +#define TEST_MACRO(xx) do { \ + const STRUCT_SECTION_ITERABLE(settings_handler_static, \ + name) \ +} while (0) + +static inline int inline_static_fn(void) +{ + return 0; +} + +inline static int static_inline_fn(void) +{ + return 0; +} + +static ALWAYS_INLINE int always_inline_fn(void) +{ + return 0; +} + +__syscall void syscall(void); + +static inline void z_impl_syscall(void) +{ + /* empty */ +} + +/* macro function that was parsed incorrectly (NCSDK-10918) */ +#define SETTINGS_STATIC_HANDLER_DEFINE(_hname, _tree, _get, _set, _commit, \ + _export) \ + const Z_STRUCT_SECTION_ITERABLE(settings_handler_static, \ + settings_handler_ ## _hname) = { \ + .name = _tree, \ + .h_get = _get, \ + .h_set = _set, \ + .h_commit = _commit, \ + .h_export = _export, \ + } +int after_macro(void); + +int extra_whitespace (void) ; + +/* typedef that should not be parsed as a function */ +typedef struct __attribute__ ((__packed__)) { +} bar_t; + +/* newline between arguments */ +int multiline_def(int arg, + int len); + +#endif /* __TEST_CODE_H */ diff --git a/tests/unity/wrap_test/src/wrap_test.c b/tests/unity/wrap_test/src/wrap_test.c new file mode 100644 index 000000000000..7568fcbe96b1 --- /dev/null +++ b/tests/unity/wrap_test/src/wrap_test.c @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause + */ +#include +#include +#include "call.h" +#include "test_code/mock_test_code.h" + +/* + * These tests verify the wrapping functionality in scripts/unity that parse + * C header files and rename functions using python and regular expressions. + */ + +void test_syscall(void) +{ + __wrap_syscall_Expect(); + + call_syscall(); +} + +void test_comment_removal_allow_url(void) +{ + TEST_ASSERT_EQUAL_STRING("https://my.page.com/somewhere", SOME_ADDR); +} + +void test_static_inline_fn(void) +{ + const int EXPECT_STATIC_INLINE = 1; + + __wrap_static_inline_fn_ExpectAndReturn(EXPECT_STATIC_INLINE); + + TEST_ASSERT_EQUAL(EXPECT_STATIC_INLINE, call_static_inline_fn()); +} + +void test_inline_static_fn(void) +{ + const int EXPECT_INLINE_STATIC = 2; + + __wrap_inline_static_fn_ExpectAndReturn(EXPECT_INLINE_STATIC); + + TEST_ASSERT_EQUAL(EXPECT_INLINE_STATIC, call_inline_static_fn()); +} + +void test_always_inline_fn(void) +{ + const int EXPECT_ALWAYS_INLINE = 3; + + __wrap_always_inline_fn_ExpectAndReturn(EXPECT_ALWAYS_INLINE); + + TEST_ASSERT_EQUAL(EXPECT_ALWAYS_INLINE, call_always_inline_fn()); +} + +void test_after_macro(void) +{ + const int EXPECT_AFTER_MACRO = 4; + + __wrap_after_macro_ExpectAndReturn(EXPECT_AFTER_MACRO); + + TEST_ASSERT_EQUAL(EXPECT_AFTER_MACRO, call_after_macro()); +} + +void test_extra_whitespace(void) +{ + const int EXPECT_EXTRA_WHITESPACE = 5; + + __wrap_extra_whitespace_ExpectAndReturn(EXPECT_EXTRA_WHITESPACE); + + TEST_ASSERT_EQUAL(EXPECT_EXTRA_WHITESPACE, call_extra_whitespace()); +} + +void test_multiline_def(void) +{ + const int EXPECT_MULTILINE_DEF = 6; + + __wrap_multiline_def_ExpectAndReturn(1, 2, EXPECT_MULTILINE_DEF); + + TEST_ASSERT_EQUAL(EXPECT_MULTILINE_DEF, call_multiline_def(1, 2)); +} + +/* It is required to be added to each test. That is because unity is using + * different main signature (returns int) and zephyr expects main which does + * not return value. + */ +extern int unity_main(void); + +void main(void) +{ + (void)unity_main(); +} diff --git a/tests/unity/wrap_test/testcase.yaml b/tests/unity/wrap_test/testcase.yaml new file mode 100644 index 000000000000..c22fce3f21a6 --- /dev/null +++ b/tests/unity/wrap_test/testcase.yaml @@ -0,0 +1,5 @@ +tests: + unity.wrap_test: + tags: wrap_test + integration_platforms: + - native_posix