Re: [PATCH v3 0/2] kunit: add macro to allow conditionally exposing static symbols to tests

From: John Johansen
Date: Tue Dec 06 2022 - 21:43:21 EST


On 12/6/22 17:40, Rae Moar wrote:
Currently in order to test a static function, tests must be included in the
same translation unit as the function. However, this can cause issues with
including implementation and test code in the same file. As an alternative,
the first patch in this series creates a macro that will set a function to
be static or not depending on whether CONFIG_KUNIT is enabled. This allows
the function to be visible during testing and static otherwise.

As an example, the current status quo to test static functions is:

=== test.c ===

static void test_case(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, my_func_to_test(), 2);
}

Then the tests are included in the implementation file as a workaround to
the issue of testing static functions:

=== implementation.c ===

static int my_func_to_test() {...}
...
#include "test.c"

Instead, the function could be defined with this new macro:

=== implementation.c ===

VISIBLE_IF_KUNIT int my_func_to_test() {...}

The first patch also creates a macro that will export a symbol into a kunit
testing namespace only if CONFIG_KUNIT is enabled. This follows the logic
above and allows symbols to be conditionally exported based on the testing
status.

The second patch in the series updates the policy_unpack test in AppArmor
to show an example of how to use both of these macros in order to address
the issue of testing static functions. Additionally, the patch allows the
policy_unpack test to be built as a module.

Changes since v2:
- Add mention of namespacing symbols to the commit message of the
second patch.
- Change module name in the second patch from policy_unpack_test to
apparmor_policy_unpack_test.

Changes since v1:
- Changed the namespace of exported symbols for the apparmor
policy_unpack_test by adding the aa_ prefix.
- Separated the documentation comments for macros in
include/kunit/visibility.h.
- Changed copyright date and author for include/kunit/visibility.h.

Rae Moar (2):
kunit: add macro to allow conditionally exposing static symbols to
tests
apparmor: test: make static symbols visible during kunit testing

include/kunit/visibility.h | 33 +++
security/apparmor/Kconfig | 4 +-
security/apparmor/Makefile | 3 +
security/apparmor/include/policy_unpack.h | 50 +++++
security/apparmor/policy_unpack.c | 238 ++++++++++------------
security/apparmor/policy_unpack_test.c | 69 ++++---
6 files changed, 229 insertions(+), 168 deletions(-)
create mode 100644 include/kunit/visibility.h


base-commit: 0f08f3e2a0186dfb8e33cb46105228eb18448a0e

thanks Rae,

looks good to me, David unless you tell me otherwise I assume this is
still going in via the kselftest/kunit tree.