feat: add unit tests for vc-task #2665
Conversation
felixfaisal
requested review from
a team,
BillyWooo,
kziemianek,
higherordertech and
Kailai-Wang
felixfaisal
changed the title
| @@ -68,6 +68,7 @@ RUN \ | |||
| fi | |||
|
|
|||
| RUN cargo test --release --features development | |||
| RUN cargo test -p lc-vc-task-receiver -- --ignored --test-threads 1 | |||
There was a problem hiding this comment.
If there is no rush i suggest debugging --test-threads before this PR gets merged.
There was a problem hiding this comment.
The reason why I'm choosing to debug this in another issue, is because the most likely reason why it fails is because of the two globals we are using i.e. GLOBAL_MOCK_RPC_API and GLOBAL_MOCK_AUTHOR_API, it is something being used for only the unit testing, So I prefer doing it another PR. I would have to use a completely different mechanism to capture the values, and I think I should take a dig at it in a different/follow-up PR.
| let sender_guard = GLOBAL_MOCK_RPC_API.lock().unwrap(); | ||
| let sender = &*sender_guard; | ||
| sender.as_ref().expect("Not yet initialized").send(encoded_value).unwrap(); | ||
| // Box::pin(ready(Ok([0u8; 32].into()))) |
| let top_calls_receiver = init_global_mock_author_api().unwrap(); | ||
| let rpc_calls_receiver = init_global_rpc_api().unwrap(); | ||
|
|
||
| thread::sleep(Duration::from_secs(2)); |
There was a problem hiding this comment.
What's the purpose of this sleep ?
There was a problem hiding this comment.
This is a sanity check to make sure things are initialised.
| pub static ref SHIELDING_KEY: ShieldingCryptoMock = ShieldingCryptoMock::default(); | ||
| } | ||
|
|
||
| pub fn init_global_mock_author_api() -> Result<Receiver<Vec<u8>>> { |
There was a problem hiding this comment.
What do you think about keeping only mock definitions here and removing test util functions and functions that initializes tests context (move everything to test.rs and remove this file) ?
There was a problem hiding this comment.
Yeah I can do that, I followed the pattern of unit tests in pallets.
| use crate::mock::*; | ||
|
|
||
| #[test] | ||
| #[ignore] |
There was a problem hiding this comment.
Why is this test case ignored ?
There was a problem hiding this comment.
Yeah this is so that it is not run with all the tests, and only run with the test-threads 1
There was a problem hiding this comment.
This means if we add more ignored tests to this package they will be run alongside ones added in this PR, is this fine ?
There was a problem hiding this comment.
Yeah that should be fine imo, they will be run with cargo test -p lc-vc-task-receiver -- --ignored --test-threads 1
So this PR aims to mock some parts of the
vc-taskthread and test. I've createdmock.rsto create all the mock instances: ;A few problems I want to address in different issues:
--test-threads 1, if not it fails withSendError. I want to debug this in another issue, for now I'm running these tests on a single thread. Not related totee-devA1as all others require network calls, So I'm only testing theA1assertion, and secondly I'm not asserting if the credential str produced is correct or not.