On 9/23/22 11:15, Thomas Zimmermann wrote:
Hi
Am 22.09.22 um 16:25 schrieb Maxime Ripard:
drm_connector_pick_cmdline_mode() is in charge of finding a proper
drm_display_mode from the definition we got in the video= command line
argument.
Let's add some unit tests to make sure we're not getting any regressions
there.
Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index bbc535cc50dd..d553e793e673 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -1237,3 +1237,7 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
return ret;
}
EXPORT_SYMBOL(drm_client_modeset_dpms);
+
+#ifdef CONFIG_DRM_KUNIT_TEST
+#include "tests/drm_client_modeset_test.c"
+#endif
I strongly dislike this style of including source files in each other.
It's a recipe for all kind of build errors. Can you do something else?
This seems to be the convention used to test static functions and what's
documented in the Kunit docs: https://kunit.dev/third_party/kernel/docs/tips.html#testing-static-functions
I agree with you that's not ideal but I think that consistency with how
it is done by other subsystems is also important.
As the tested function is an internal interface, maybe export a wrapper
if tests are enabled, like this:
#ifdef CONFIG_DRM_KUNIT_TEST
struct drm_display_mode *
drm_connector_pick_cmdline_mode_kunit(drm_conenctor)
{
return drm_connector_pick_cmdline_mode(connector)
}
EXPORT_SYMBOL(drm_connector_pick_cmdline_mode_kunit)
#endif
The wrapper's declaration can be located in the kunit test file.
But that's also not nice since we are artificially exposing these only
to allow the static functions to be called from unit tests. And would
be a different approach than the one used by all other subsystems...
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature