From 11073a12c80cab87fd9d0f7798a8a66a9d0e5791 Mon Sep 17 00:00:00 2001 From: Jacob Gamble Date: Sat, 30 May 2026 14:20:05 -0500 Subject: [PATCH] Update CONTRIBUTING.md to clarify test flag policy Clarify that test flags should be minimal hooks into internal functionality, not contain test logic. All test logic (assertions, validation) belongs in Bash test scripts in tests/. This addresses the ambiguity mentioned in issue 109 regarding the distinction between test hooks in main.cpp and test scripts. --- CONTRIBUTING.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2037892..e455380 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -94,10 +94,19 @@ Exception only if it is API calling code which is hard to test deterministically. ### Adding new flags to the CLI for testing -Adding new flags to the CLI for testing is allowed and ENCOURAGED! For example -for the skill stuff, we added a flag -D that just dumps all skill directories -found. This is useless to the end user, but lets us test the fact that our -skill directory search was working. +Adding new flags to the CLI for testing is allowed and ENCOURAGED! These +flags serve as **hooks into internal functionality** to enable testing of +modules that don't have a natural CLI entry point. + +For example, for the skill stuff, we added a flag -D that just dumps all skill +directories found. This is useless to the end user, but lets us test the fact +that our skill directory search was working. + +**Test flags should be minimal hooks:** +- They should accept input, call the internal function, and output the result +- They should NOT contain test logic (assertions, validation, etc.) +- All test logic belongs in the Bash test scripts in `tests/` +- The flag's only job is to bridge CLI arguments to the internal function If you add a new flag for testing you don't need to update the README.md (but updating the -h message is still mandatory).