Update tracing setup and add more integration tests #50
Conversation
| tracing::subscriber::set_global_default(subscriber)?; | ||
| let _ = tracing_subscriber::fmt() | ||
| .try_init() | ||
| .map_err(|e| debug!("Failed to initialize global tracing: {}", e)); |
There was a problem hiding this comment.
We had an issue where tests couldn’t run in parallel (Multiple calls to this run fn) because the global tracing subscriber was already set. This fix checks if the global config is already set and only sets it if it hasn’t been initialized.
| assert_eq!(our_response, their_response); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| #[serial] |
There was a problem hiding this comment.
I think we should run all these tests one after the other and always make sure the databases are clean before each test. That way, commands like keys will give more accurate results. In the future, we could also consider using a different DB for each test, though I’m not sure if it’s worth it.
There was a problem hiding this comment.
I like the idea of a different DB. For Redis side we could use different virtual DBs on the same instance, for Rustdis we could spin up different "instances" on different ports. Wdyt?
| // p.cmd("SET") | ||
| // .arg("incr_by_key_4") | ||
| // .arg("234293482390480948029348230948"); | ||
| // p.cmd("INCRBY").arg("incr_by_key_4").arg(1); |
There was a problem hiding this comment.
@gillchristian I know you don't like commented code but I don't want to deal with these errors right now. We are returning the correct error message but the formats are a bit different. I can uncomment this and make it fail.
There was a problem hiding this comment.
Yeah, not a fan of commented-out code. I would much rather have the test fail as a TODO that we have to fix.
| type Response = ( | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| Value, | ||
| ); | ||
|
|
||
| test_compare::<Response>(|p| { |
There was a problem hiding this comment.
| type Response = ( | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| Value, | |
| ); | |
| test_compare::<Response>(|p| { | |
| test_compare::<Vec<Value>>(|p| { |
| type Response = (Value, Value, Value, Value, Value, Value); | ||
|
|
There was a problem hiding this comment.
| type Response = (Value, Value, Value, Value, Value, Value); |
| type Response = (Value, Value, Value, Value, Value, Value); | ||
|
|
||
| test_compare::<Response>(|p| { |
There was a problem hiding this comment.
| type Response = (Value, Value, Value, Value, Value, Value); | |
| test_compare::<Response>(|p| { | |
| test_compare::<Vec<Value>>(|p| { |
| type Response = (Value, Value, Value, Value, Value, Value, Value); | ||
|
|
||
| test_compare::<Response>(|p| { |
There was a problem hiding this comment.
| type Response = (Value, Value, Value, Value, Value, Value, Value); | |
| test_compare::<Response>(|p| { | |
| test_compare::<Vec<Value>>(|p| { |
| type Response = (Value, Value, Value, Value, Value, Value); | ||
|
|
||
| test_compare::<Response>(|p| { |
There was a problem hiding this comment.
| type Response = (Value, Value, Value, Value, Value, Value); | |
| test_compare::<Response>(|p| { | |
| test_compare::<Vec<Value>>(|p| { |
| type Response = (Value, Value, Value, Value, Value, Value); | ||
|
|
||
| test_compare::<Response>(|p| { |
There was a problem hiding this comment.
| type Response = (Value, Value, Value, Value, Value, Value); | |
| test_compare::<Response>(|p| { | |
| test_compare::<Vec<Value>>(|p| { |
This PR adds: