Mocking Context and ValkeyString so it can be unit tested#228
Mocking Context and ValkeyString so it can be unit tested#228dmitrypol wants to merge 4 commits intovalkey-io:mainfrom
Conversation
0e6846e to
3971fc4
Compare
stockholmux
left a comment
There was a problem hiding this comment.
I'm far from being even Rust competent, but this looks okay. However, broadly, if you want people to use these features they need to be very explicitly documented and front and centre in your README.
3971fc4 to
18fe1d1
Compare
PDXKimani
left a comment
There was a problem hiding this comment.
Hmm. Thinking about this, I think my main question is as to what putting the mock traits/derives within the crate itself is providing here. So far the only consumer I'm seeing are in the examples. I think it's fine to pattern-set how one can use a dependency injection approach to "mock" the module interface, but I think the approach on display currently doesn't actually require any internal access to the module crate? That is, if the mock definitions lived externally to the module crate, it wouldn't affect the examples' usage at all.
If we intend to use ContextTrait internally to the module crate to allow consumers to control dependency injection for the module itself, that's a bit of different story. If the module layer has sufficient complexity over the underlying C API that there'd be value in doing so, then I would see the argument for including that trait in the crate, though I still think I'd leave the decision of what mocking framework to use/how to implement a mocked version up to the consumer.
With regards to the mechanical usage of mocking in the examples, that seems fine.
|
to clarify I want to use this Mock context in various modules that I am building. Currently I am unable to test functions that require passing &Context. This will allow me to throughly unit test that code. |
18fe1d1 to
68570f7
Compare
updated examples to include tests Signed-off-by: dmitrypol <dmitrypol@users.noreply.github.com>
7847040 to
7eae477
Compare
updated examples with unit tests Signed-off-by: dmitrypol <dmitrypol@users.noreply.github.com>
7eae477 to
cf592ab
Compare
f586930 to
5d6fff3
Compare
a98b7f0 to
46f6fc2
Compare
updated examples to use it Signed-off-by: dmitrypol <dmitrypol@users.noreply.github.com>
46f6fc2 to
3e040b7
Compare
KarthikSubbarao
left a comment
There was a problem hiding this comment.
Thanks @dmitrypol , The main comment I have would be to have each "Context" like struct mocked internally, without requiring a user to switch to a new trait. Each Context can have a startmock / shim invocation at the beginning of unit test runs where it points the Raw Module APIs to mocked APIs. By having this startmock / shim automatically executed from the unit test execution, users will not have to refactor their business logic, and will get the mock functionality to their benefit
Introduces a ContextTrait abstraction over Context and a mockall-generated MockContext so command handlers can be unit-tested without a live Valkey server.
New mock infrastructure (src/context/mock/):
ContextTrait + MockContext — logging, call, module options, server version, auth,
and the full client API (get_client_id, get_client_name*, set_client_name*,
get_client_username*, get_client_cert, get_client_info*, get_client_ip*,
deauthenticate_and_close_client*, config_get).
CommandFilterCtxTrait + MockCommandFilterCtx — args_count, arg_get,
arg_get_try_as_str, cmd_get_try_as_str, get_all_args_wo_cmd, arg_replace, arg_insert,
arg_delete, feature-gated get_client_id.
InfoContextTrait + MockInfoContext — build_one_section.
Mock* — generated via #[mockall::automock], gated behind cfg(test) or the new test-mocks feature so release builds don't pull in mockall.
Introduced redismodule_test.rs with
ValkeyString::create_for_testtest harness for ValkeyString, implemented by mocking just enough of the Valkey string API to run unit tests entirely in Rust. Will NOT impact any real codeCargo
Updated examples to use the new impl for real code and mocks /
ValkeyString::create_for_testfor unit tests