Hi,
On Thu, Feb 02, 2023 at 01:22:01PM +0100, Thomas Zimmermann wrote:
> Am 02.02.23 um 12:03 schrieb Maxime Ripard:
> > Commit 8fc0380f6ba7 ("drm/client: Add some tests for
> > drm_connector_pick_cmdline_mode()") was meant to introduce unit tests
> > for the static drm_connector_pick_cmdline_mode() function.
> >
> > In such a case, the kunit documentation recommended to import the tests
> > source file directly from the source file with the static function to
> > test.
> >
> > While it was working, it's generally frowned upon. Fortunately, commit
> > 9c988fae6f6a ("kunit: add macro to allow conditionally exposing static
> > symbols to tests") introduced macros to easily deal with that case. We
> > can thus remove our include and use those macros instead.
>
> I like that this include statements is going away.
Yeah, when I saw that it was now available, I remembered you really
didn't like it :)
> But changing symbol visibility for tests is likewise awkward.
>
> Maybe i'm askin gtoo miuch for this simple patch, but can't we have a helper
> macro that generates an exported wrapper for Kunit tests? Something like
> this:
>
> EXPORT_KUNIT_WRAPPER(struct drm_display_mode *\
> drm_connector_pick_cmdline_mode,
> struct drm_connector *connector);
>
> which then generates something like this:
>
> struct drm_display_mode * drm_connector_pick_cmdline_mode_kunit(
> struct drm_connector *connector)
> {
> return drm_connector_pick_cmdline_mode(connector);
> }
>
> I know that the macro for generating this code is more complex than
> illustrated here. But this solution separates Kunit and functions cleanly.
> The static functions that are exported for Kunit testing still need to be
> declared in a header file. That could also be done via such a macro.
I mean, I guess we could do that, but what's the point? I don't really
get what that wrapper brings to the table.
Also, this deviates from the existing practice we had for selftests and
EXPORT_SYMBOL_FOR_TESTS_ONLY
Maxime
Attachment:
signature.asc
Description: PGP signature
