mocking.rst: add sections on comptime and linktime redirection
Give examples of using #ifdef and __weak symbols to add a layer of
indirection to function calls.
Also show how we can combine link/compile-time indirection w/ runtime
indirection to have zero-overhead for normal builds, while still
limiting the scope of the changes.
Change-Id: Iaccc1c1708ebbf9cb2662743aaee8fd3ccb38b03
Signed-off-by: Daniel Latypov <dlatypov@google.com>
diff --git a/mocking.rst b/mocking.rst
index 2791e87..7f17c9e 100644
--- a/mocking.rst
+++ b/mocking.rst
@@ -197,32 +197,228 @@
Compile time
------------
-TODO(dlatypov@google.com): write me
+This involves using compiler directive, macros, etc. to change the code when
+compiling KUnit tests, or for your specific test. E.g. a straightforward
+approach could be
+
+.. code-block:: c
+
+ void send_data_to_hardware(const char *str)
+ {
+ #ifdef CONFIG_MY_KUNIT_TEST /* or some other, more generic check */
+ test_send_data(str);
+ return;
+ #endif
+
+ /* real implementation */
+ }
+
+
+
+This pattern is generally useful and recommended `here
+<third_party/kernel/docs/tips.html#injecting-test-only-code>`__ for injecting
+other kinds of test-only code.
+
+.. note::
+ This example makes it so that ``send_data_to_hardware()`` no longer
+ works for the whole kernel, including other tests if they happen to
+ compiled along with ``CONFIG_MY_KUNIT``, even if it's set as
+ ``CONFIG_MY_KUNIT_TEST=m`` (!). See :ref:`hybrid-approaches` for a
+ discussion on how to mitigate that.
Pros:
~~~~~
-- TODO
+- Also easy to understand.
+- No runtime overhead, unlike the option above.
Cons:
~~~~~
-- TODO
+- Requires invasive changes to the function we're trying to stub as opposed to
+ the coder-under-test itself.
+
+ - So it's not suitable if you don't own ``send_data_to_hardware()``.
+
+- We don't want to pollute normal code by doing this at a large scale.
+
+ - So it probably should only be used sparingly in narrow contexts, e.g.
+ in static functions.
+
Link time (__weak symbols)
--------------------------
-TODO(dlatypov@google.com): write me
+We can avoid refactoring to the code-under-test and only have minimal changes
+to ``send_data_to_hardware()`` by pushing the indirection into the linker.
+
+More specifically, we can make use of "weak symbols", which would allow tests
+to define their own definitions and override the original
+``send_data_to_hardware()``, e.g.
+
+.. code-block:: c
+
+ void __weak send_data_to_hardware(const char *str)
+ {
+ /* real implementation */
+ }
+
+ /* In test file */
+ /* Since the real function is __weak, we can override it here. */
+ void send_data_to_hardware(const char *str)
+ {
+ /* fake implementation */
+ }
+
+We can minimize the risk of accidentally overriding functions by using a macro
+like
+
+.. code-block:: c
+
+ /* Now we can mark functions __weak only while building tests */
+ #ifdef CONFIG_KUNIT
+ #define __mockable __weak
+ #else
+ #define __mockable
+ #endif
+
Pros:
~~~~~
-- TODO
+- No runtime overhead.
+- No changes needed to the code-under-test, ``func_under_test()``.
+- And overall, very minimal changes needed to non-test code.
Cons:
~~~~~
-- TODO
+- Initial feedback when something similar was included in the initial KUnit RFC
+ is that this is adding more complexity.
+
+ - ``ftrace`` and friends exist and allow for patching binaries.
+ - Note that this is a much more stripped-down version of what the KUnit
+ RFC called "function mocking," so it's not as big of a concern.
+
+- **Not possible to limit the scope of the redirection.** The real definition
+ is discarded by the linker.
+
+ - Can also only have one fake definition at any time.
+ - See :ref:`hybrid-approaches`.
+
+- Won't work with tests built as modules.
+
+- More complicated, harder to understand, and less explicit.
+
+ - E.g. if you forgot to include the replacement definition in the
+ compilation unit, the code will happily call the original one without
+ any warning or indication.
+ - Your test might want the real function to be called, so building your
+ test together with someone else's might cause failures if they
+ redirected it.
+
+.. _hybrid-approaches:
+
+Hybrid approaches (limiting scope)
+----------------------------------
+
+Summarizing, the main issue with the two approaches above is that they have
+effects globally and for the full life of the kernel, not just during tests.
+
+How can we get around this? Why, of course, by adding another layer of
+indirection. Using some cleverness, we can use runtime indirection while
+building tests, but not pay the cost for normal builds.
+
+Say we want to redirect/intercept calls to ``kmalloc()``. Too many callsites
+need to use it so that stubbing it out is unwise (and would break KUnit
+itself). So we can introduce a ``__weak`` wrapper around it like so:
+
+.. code-block:: c
+
+ #ifdef CONFIG_KUNIT
+ #define __mockable_wrapper __weak
+ #else /* avoid the performance overhead for real builds */
+ #define __mockable_wrapper __always_inline
+ #endif
+
+ void __mockable_wrapper kmalloc_wrapper(size_t n, gfp_t gfp)
+ {
+ return kmalloc(n, gfp);
+ }
+
+
+This adds more boilerplate, but lets us manage the scope of the code affected
+by the redirection. We can also limit the temporal scope of the redirection if
+our replacement is defined like:
+
+.. code-block:: c
+
+ /* In the test file. This is the definition that overrides the __weak version */
+ static void* (*kmalloc_ptr)(size_t n, gfp_t gfp) = kmalloc;
+ void* kmalloc_wrapper(size_t n, gfp_t gfp) {
+ return kmalloc_ptr(n, gfp);
+ }
+
+ /* elsewhere in test case: use a fake implementation and then revert back */
+ kmalloc_ptr = my_fake_kmalloc;
+ KUNIT_EXPECT_EQ(some_test_function(), 42);
+ kmalloc_ptr = kmalloc;
+
+Note that we can do the same thing using compile-time indirection
+
+.. code-block:: c
+
+ #ifdef CONFIG_MY_KUNIT_TEST /* or some other, more generic check */
+ static void* (*kmalloc_ptr)(size_t n, gfp_t gfp) = kmalloc;
+ #endif
+
+ void* kmalloc_wrapper(size_t n, gfp_t gfp) {
+ #ifdef CONFIG_MY_KUNIT_TEST
+ return kmalloc_ptr(n, gfp);
+ #endif
+ return kmalloc(n, gfp);
+ }
+
+ /* in test file, use the fake for one call */
+ kmalloc_ptr = my_fake_kmalloc;
+ KUNIT_EXPECT_EQ(some_test_function(), 42);
+ kmalloc_ptr = kmalloc;
+
+
+See :ref:`managing-state` for more details on cleanly handling state in test
+doubles. In particular, one could use "named resources" instead of global
+variables like ``kmalloc_ptr``. That approach would also be thread-safe, unlike
+this example.
+
+Pros:
+~~~~~
+
+- No runtime overhead outside of tests.
+
+ - For the link-time based approach ``__always_inline`` should eliminate
+ the potential function call overhead.
+ - With the compile-time approach, the code is unchanged unless the
+ relevant tests are being built.
+
+- Able to limit the scope of the redirection to code we care about.
+- Are also able to limit how long the redirection is in place.
+
+Cons:
+~~~~~
+
+- The code-under-test (``func_under_test()``) needs to be changed to use this
+ new wrapper, somewhat hurting legibility.
+
+- Requires a decent amount of boilerplate for every single function we might
+ want to stub.
+
+ - Probably best reserved for only a few key functions. We could have
+ shared implementations for some, e.g. ``kmalloc()``, if they're
+ flexible enough (e.g. allow assigning arbitrary function pointers
+ like in the second example).
+
+- Inherits issues of the other approach, e.g. ``__weak`` won't work with
+ modules, etc.
Binary-level (ftrace et. al)
----------------------------
@@ -239,9 +435,11 @@
- TODO
-TODO(dlatypov@google.com): include discussion on global functions/general statefulness.
TODO(dlatypov@google.com): include section on worked example use cases.
+
+.. _managing-state:
+
Storing and accessing state for fakes/mocks
===========================================
