Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Make block-streamer unit-testable and add tests #442

Merged
merged 23 commits into from
Dec 14, 2023

Conversation

morgsmccauley
Copy link
Collaborator

@morgsmccauley morgsmccauley commented Dec 11, 2023

This PR is a combination of refactoring + testing, the former enabling the latter

Mock impl instead of trait

Originally, traits such as S3ClientTrait were used to abstract the behaviour of I/O related structs, allowing for the injection of mock implementations via automock. This was fine, but lead to very verbose syntax which needed to be propagated throughout the codebase where ever it was used.

Now, the impl itself is mocked, and either the mock or real implementation is returned dependant on the cfg(test) flag. This means we no longer need to worry about generics and can instead use the struct itself, as we would with non-mocked code.

The trade-off here is that rust now thinks the 'real' code is dead/unused as test is enabled by default.

Mocking near-lake-framework

aws exposes an HTTP client which can be configured with mock request/response values. aws-sdk-s3 can be configured to use this client, creating a mock client. Injecting this config into near-lake-framework allows us to create deterministic outputs, enabling unit-testing.

aws_config is now taken as a parameter where relevant so that we can mock near-lake-framework in out tests. The functions for achieving this have been placed in test_utils so they can be reused.

Test BlockStreamerService and BlockStream

With RedisClient, S3Client, and Lake now mockable - tests have been added for these structs. I've also added a Github action which runs on every PR. All raw block data has been added to data/, which makes up the majority of the diff in this PR.

@morgsmccauley morgsmccauley changed the base branch from main to 420-expose-endpoint-to-control-streams December 11, 2023 00:25
@morgsmccauley morgsmccauley force-pushed the test/block-streamer branch 2 times, most recently from 21ebb38 to c77a532 Compare December 11, 2023 02:55
@morgsmccauley morgsmccauley changed the title test/block streamer test: Make block-streamer unit-testable and add tests Dec 11, 2023
tokio-util = "0.7.10"
tokio-stream = "0.1.14"
tonic = "0.10.2"
wildmatch = "2.1.1"

near-lake-framework = "0.7.1"
near-lake-framework = { git = "https://github.com/near/near-lake-framework-rs", branch="chore/upgrade-aws-0.7.x"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will pin to the specific version once near/near-lake-framework-rs#94 is merged

@morgsmccauley morgsmccauley marked this pull request as ready for review December 11, 2023 08:13
@morgsmccauley morgsmccauley requested a review from a team as a code owner December 11, 2023 08:13
@morgsmccauley morgsmccauley linked an issue Dec 11, 2023 that may be closed by this pull request
Copy link
Collaborator

@darunrs darunrs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool stuff!

@morgsmccauley morgsmccauley force-pushed the 420-expose-endpoint-to-control-streams branch from 9322584 to 2d570f0 Compare December 14, 2023 00:11
Base automatically changed from 420-expose-endpoint-to-control-streams to main December 14, 2023 00:13
@morgsmccauley morgsmccauley merged commit 28bc9f3 into main Dec 14, 2023
6 checks passed
@morgsmccauley morgsmccauley deleted the test/block-streamer branch December 14, 2023 00:21
@morgsmccauley morgsmccauley mentioned this pull request Dec 19, 2023
@morgsmccauley morgsmccauley self-assigned this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants