On Wed, Dec 7, 2022 at 10:43 AM John Johansen
<john.johansen@xxxxxxxxxxxxx> wrote:
>
> 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.
>
Yup, both of these are looking good to me (and work fine on my machine).
I've added these to the queue for the kselftest/kunit tree. There's a
chance they won't make it into 6.2, but if not, they should be the
first things in the 6.3 branch.
Thanks very much,
-- David
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
