diff --git a/design.md b/design.md index d5fb42b..ea5f390 100644 --- a/design.md +++ b/design.md @@ -327,6 +327,7 @@ This write-side component is designed to integrate with: - **Configuration**: All configuration is TOML-based using `config.toml` (see `config.md`) - **Testing Strategy**: Run unit tests, benchmarks, and debug tools before submitting changes +- **Test Design**: Prefer testing through public interfaces rather than implementation details (see `style.md` for detailed testing guidelines) - **Build System**: CMake generates gperf hash tables at build time; always use ninja - **Test Synchronization**: - **ABSOLUTELY NEVER use sleep(), std::this_thread::sleep_for(), or any timeout-based waiting in tests** diff --git a/style.md b/style.md index 2e6c7cf..759047f 100644 --- a/style.md +++ b/style.md @@ -379,6 +379,28 @@ TEST_CASE("ArenaAllocator basic allocation") { } ``` +### Test Design Principles +- **Prefer testing through public interfaces** - focus on observable behavior rather than implementation details +- **Test the contract, not the implementation** - validate what the API promises to deliver +- **Avoid testing private methods directly** - if private functionality needs testing, consider if it should be public or extracted +- **Integration over isolation** - test components working together when practical +- **Mock only external dependencies** - prefer real implementations for internal components +```cpp +// Good: Testing through public API +TEST_CASE("Server accepts connections") { + auto config = Config::defaultConfig(); + auto handler = std::make_unique(); + auto server = Server::create(config, std::move(handler)); + + // Test observable behavior - server can accept connections + auto result = connectToServer(server->getPort()); + CHECK(result.connected); +} + +// Avoid: Testing internal implementation details +// TEST_CASE("Server creates epoll instance") { /* implementation detail */ } +``` + ### Test Synchronization - **NEVER use timeouts** or sleep-based synchronization - **Deterministic synchronization only:** diff --git a/tests/test_http_handler.cpp b/tests/test_http_handler.cpp index cc97913..84cc5c0 100644 --- a/tests/test_http_handler.cpp +++ b/tests/test_http_handler.cpp @@ -68,7 +68,7 @@ TEST_CASE("HttpHandler route parsing") { } } -TEST_CASE("HttpHandler route parsing functionality") { +TEST_CASE("HttpHandler route parsing edge cases") { // Test just the static route parsing method since full integration testing // would require complex Connection setup with server dependencies diff --git a/tests/test_server_connection_return.cpp b/tests/test_server_connection_return.cpp index c73d3a9..fb377ca 100644 --- a/tests/test_server_connection_return.cpp +++ b/tests/test_server_connection_return.cpp @@ -35,7 +35,8 @@ public: } }; -TEST_CASE("Echo server with connection ownership transfer") { +TEST_CASE( + "Server correctly handles connection ownership transfer via pipeline") { weaseldb::Config config; config.server.io_threads = 1; config.server.epoll_instances = 1;