Initial prototype of test refactor by padelsbach · Pull Request #331 · wolfSSL/wolfHSM

padelsbach · 2026-04-08T16:35:43Z

Refactor top level test modules with the following:

New test runner
Unified connection logic (helper)
Cleaner integration of sequenced tests

See more in the README.md

bigbrett

Overall,as discussed, looks great. A few things I wanted to point out and some architectural questions that should be addressed.

Also, can we get a code coverage comparison set up in CI so we can track the coverage metrics between existing test and new harness?

bigbrett · 2026-04-30T16:31:02Z

+int whTestGroup_ResetServer(whServerContext* server);
+int whTestGroup_ResetClient(whClientContext* client);


should this be whTestPort_ResetXXX() following naming convention based on file/module?

bigbrett · 2026-04-30T16:34:11Z

+/*
+ * One-shot declaration of a test's forward prototype AND its weak
+ * skip implementation. wh_test_list.c contains a `WH_TEST_DECL(name);`
+ * line for every test in the registry. If the real test's feature
+ * gate is on, its strong definition in the test's .c file overrides
+ * this stub at link time; otherwise the stub wins and the test
+ * surfaces as SKIPPED.
+ *
+ * The trailing "struct ... whTest_decl_dummy_" lets callers end
+ * the invocation with a ';' (a bare "};" after a function body
+ * is ill-formed under strict C90); the dummy tag swallows it.
+ */


Read this 3 times and it still wasn't clear what it is for or what it does. How about this:

Suggested change

/*

* One-shot declaration of a test's forward prototype AND its weak

* skip implementation. wh_test_list.c contains a `WH_TEST_DECL(name);`

* line for every test in the registry. If the real test's feature

* gate is on, its strong definition in the test's .c file overrides

* this stub at link time; otherwise the stub wins and the test

* surfaces as SKIPPED.

*

* The trailing "struct ... whTest_decl_dummy_" lets callers end

* the invocation with a ';' (a bare "};" after a function body

* is ill-formed under strict C90); the dummy tag swallows it.

*/

/*

* Declares a test prototype and provides a weak default stub.

* wh_test_list.c uses WH_TEST_DECL(name) for every registered test.

*

* If the real test is compiled in, its strong definition overrides

* this stub at link time. Otherwise, the stub remains and the test

* is reported as SKIPPED.

*

* The trailing dummy struct lets callers safely end the macro with

* a semicolon, which avoids issues under strict C90.

*/

bigbrett · 2026-04-30T16:36:35Z

+/*
+ * Per-group registries, defined in wh_test_list.c. Each group gets
+ * its own array so the runner walks only the relevant tests and
+ * doesn't carry a group tag per row.
+ */


Suggested change

/*

* Per-group registries, defined in wh_test_list.c. Each group gets

* its own array so the runner walks only the relevant tests and

* doesn't carry a group tag per row.

*/

/*

* Per-group registries, defined in wh_test_list.c. Each group gets

* its own array so the runner walks only the relevant tests.

*/

bigbrett · 2026-04-30T16:39:02Z

+/*
+ * Portable weak-linkage attribute. wh_test_list.c
+ * emits a weak stub for every discovered test; the real test's
+ * strong definition (compiled in when its feature gate is on)
+ * overrides the stub at link time. When the gate is off the test
+ * source compiles to nothing and the weak stub wins, returning
+ * WH_TEST_SKIPPED at runtime.
+ *
+ * Usage:
+ *   WH_TEST_WEAK(foo) int foo(void* ctx) { ... }
+ *
+ * The name argument is only used by toolchains whose weak-symbol
+ * mechanism is a pragma rather than a prefix attribute; others
+ * ignore it. Spellings covered:
+ *   GCC, Clang, armclang, armcc, TI  -> __attribute__((weak))
+ *   IAR                               -> __weak
+ *   Renesas CC-RH / CC-RL / CC-RX    -> _Pragma("weak <name>")
+ *
+ * If a port's toolchain isn't covered here, add it rather than
+ * silently falling back -- a no-op WH_TEST_WEAK makes the stub
+ * strong and will cause a multiple-definition link error (or,
+ * worse, the stub wins over the real test).
+ */


Suggested change

/*

* Portable weak-linkage attribute. wh_test_list.c

* emits a weak stub for every discovered test; the real test's

* strong definition (compiled in when its feature gate is on)

* overrides the stub at link time. When the gate is off the test

* source compiles to nothing and the weak stub wins, returning

* WH_TEST_SKIPPED at runtime.

*

* Usage:

* WH_TEST_WEAK(foo) int foo(void* ctx) { ... }

*

* The name argument is only used by toolchains whose weak-symbol

* mechanism is a pragma rather than a prefix attribute; others

* ignore it. Spellings covered:

* GCC, Clang, armclang, armcc, TI -> __attribute__((weak))

* IAR -> __weak

* Renesas CC-RH / CC-RL / CC-RX -> _Pragma("weak <name>")

*

* If a port's toolchain isn't covered here, add it rather than

* silently falling back -- a no-op WH_TEST_WEAK makes the stub

* strong and will cause a multiple-definition link error (or,

* worse, the stub wins over the real test).

*/

/*

* Portable weak-symbol attribute.

*

* wh_test_list.c defines a weak stub for every test. If the real test

* is compiled in, its strong definition overrides the stub at link time.

* If not, the stub remains and returns WH_TEST_SKIPPED.

*

* Usage:

* WH_TEST_WEAK(foo) int foo(void* ctx) { ... }

*

* The name is only needed for pragma-based toolchains; others ignore it.

*

* Supported toolchains:

* GCC, Clang, ARM, TI -> __attribute__((weak))

* IAR -> __weak

* Renesas (CC-RH/RL/RX) -> _Pragma("weak <name>")

*

* Add new toolchains explicitly. A missing definition here will either

* cause link errors or let the stub override the real test.

*/

bigbrett · 2026-04-30T16:42:28Z

+int whTestGroup_RunOne(const char* name, int (*fn)(void*),
+    void* ctx)
+{
+    int rc = fn(ctx);


Null check on fn?

bigbrett · 2026-04-30T16:52:09Z

+3. Call `whTestGroup_Client(&clientCtx)`
+
+### Server
+The client unit tests can be run against the normal server application -- no special modifications required.


this line should be in the client section

bigbrett · 2026-04-30T16:54:06Z

+To add the server side unit tests:
+1. Implement `whTestGroup_ResetServer` which resets the context between tests. Can be empty.
+2. Call `whTestGroup_Server(&serverCtx)` prior to entering the main request handling loop.


start from main and talk about context init like in client section

Implement main() which declares a server context and initializes it with a config such that it can access basic platform functionality (NVM, etc.)

bigbrett · 2026-04-30T18:39:10Z

+/* Host-sim flash/NVM tests. The ramsim test exercises the
+ * RAM-based flash simulator, which is a host-sim component;
+ * the nvm_flash test wires the NVM stack to that simulator
+ * with a 1 MB buffer that's not realistic on embedded targets.
+ * Both run from the POSIX port directly until the NVM test is
+ * reworked to take a port-supplied flash fixture (then it can
+ * lift back into whTestGroup_Server). */
+int whTest_FlashWriteLock(void* ctx);
+int whTest_FlashEraseProgramVerify(void* ctx);
+int whTest_FlashUnitOps(void* ctx);
+int whTest_NvmAddOverwriteDestroy(void* ctx);


Hmmm so I guess you are treating this now as "port-specific" tests? I guess that is fine based on what they test. Perhaps should consider reorganizing or renaming at minimum - ok for now

These currently hardcode the ramsim backend, and decoupling is non-trivial. Planning to rename at that time. This is noted in the readme:

remove ramsim coupling and migrate to server group

bigbrett · 2026-04-30T18:41:46Z

+/*
+ * Caller-implemented reset hooks. The group functions invoke
+ * these between suites so the caller can reset any persistent
+ * state (NVM objects, key cache, connection state, ...) between
+ * tests. Can be empty, simply return 0.
+ */
+int whTestGroup_ResetServer(whServerContext* server);
+int whTestGroup_ResetClient(whClientContext* client);


The group functions invoke these between suites so the caller can reset any persistent state (NVM objects, key cache, connection state, ...) between tests.

Is this true? I don't see them called anywhere. IMO this should be managed internal to the runner across tests if required? Otherwise, what was the intent here? In general, tests should "clean up after themselves" if creating mutable state (NVM object, keycache entries, allocated memory, etc) but I would assume these would be called internally before running each test function as a "setup" or "teardown" operation?

Good catch, looks like this got dropped when simplifying the macros.

You're right. Idea is to give the port an opportunity to setup/teardown the context between tests without knowledge about the internals of the context.

I added the missing calls back. I don't feel too strongly about this -- I can remove if you do.

bigbrett · 2026-04-30T18:49:49Z

    - name: Build and test
      run: cd test && make clean && make -j WOLFSSL_DIR=../wolfssl && make run

+    - name: Build and test refactor


perhaps these should all just go into a new workflow to keep separate? build-and-test-refactor.yml? Also want coverage tracked between the two so should have a new coverage action for that

I put them here to promote equivalence between the refactor, reduce the possibility of them getting out of sync. Added build-and-test-refactor.yml and code-coverage-refactor.yml which mirror the originals.

bigbrett · 2026-04-30T18:51:56Z

@padelsbach oh and also, a section of the new README.md mapping old test constructs -> new test constructs would be valuable for people who need to understand the migration. Both discussing key differences at a high level, as well as mapping each individual test from its old location to its new location (this last part can be an AI table - prefer some human input for the first part)

padelsbach force-pushed the test-refactor-prototype branch 3 times, most recently from 6bd9195 to 1f06444 Compare April 8, 2026 17:17

padelsbach force-pushed the test-refactor-prototype branch 4 times, most recently from e3b29f6 to 5d9dc00 Compare April 22, 2026 16:50

padelsbach force-pushed the test-refactor-prototype branch 8 times, most recently from 5c4e0e2 to 4b6bfe6 Compare April 30, 2026 16:12

bigbrett requested changes Apr 30, 2026

View reviewed changes

bigbrett assigned padelsbach Apr 30, 2026

padelsbach force-pushed the test-refactor-prototype branch from 4b6bfe6 to 770e6ab Compare May 1, 2026 20:08

Initial prototype of test refactor

976fdca

padelsbach force-pushed the test-refactor-prototype branch from 770e6ab to 976fdca Compare May 1, 2026 21:05

padelsbach changed the title ~~Initial prototype of test refactor w/ runner, helper and test layers~~ Initial prototype of test refactor May 1, 2026

padelsbach assigned bigbrett and unassigned padelsbach May 1, 2026

		int whTestGroup_ResetServer(whServerContext* server);
		int whTestGroup_ResetClient(whClientContext* client);

Conversation

padelsbach commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bigbrett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bigbrett commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

padelsbach commented Apr 8, 2026 •

edited

Loading