Prefer testing through public APIs
This commit is contained in:
@@ -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**
|
||||
|
||||
22
style.md
22
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<TestHandler>();
|
||||
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:**
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user