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(provider): RequestsProvider of BlockchainProvider2 #10356

Merged
merged 4 commits into from
Aug 21, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Aug 15, 2024

Closes #10343

The change turned out to be more invasive than expected because I added an argument for randomly generating requests into random_block and random_block_range.

@github-actions github-actions bot added A-blockchain-tree Related to sidechains, reorgs and pending blocks A-db Related to the database C-test A change that impacts how or what we test labels Aug 15, 2024
@shekhirin shekhirin marked this pull request as ready for review August 15, 2024 22:54
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

how about otherwise making new function for when new param is used? seems like most calls take None for that param, so mainly not needed. this can be done easily by adding a third function, with the body of the current function, so that the function body of the original function, can just call my_func(arg1, arg2, .., None)

@shekhirin
Copy link
Collaborator Author

how about otherwise making new function for when new param is used? seems like most calls take None for that param, so mainly not needed. this can be done easily by adding a third function, with the body of the current function, so that the function body of the original function, can just call my_func(arg1, arg2, .., None)

yeah makes sense, agreed. Maybe we can just pass a struct instead of fields one by one, and the struct will have a Default implementation.

@shekhirin
Copy link
Collaborator Author

hmm, actually we have just one optional field now which is requests_count. I think we should leave it as-is and refactor into a struct when needed.

Merged via the queue into main with commit 3da119b Aug 21, 2024
36 checks passed
@emhane emhane deleted the alexey/blockchain-provider-requests-provider branch August 21, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-blockchain-tree Related to sidechains, reorgs and pending blocks A-db Related to the database C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test RequestsProvider implementation of BlockchainProvider2<DB>
4 participants