-
Notifications
You must be signed in to change notification settings - Fork 7
chore: add tests for the fixed count retry strategy #503
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
Conversation
11d42a8 to
1302613
Compare
Add sync and async tests for the fixed count retry strategy. Add a make target for local tests. Add a github action to run the local tests. Fix a linter error.
anitarua
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
| .PHONY: test-local | ||
| ## Run the integration tests that require Momento Local | ||
| test-local: | ||
| @poetry run pytest -m local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎗️ this only runs the momento-local specific tests. In the future we could run the entire test suite against Momento local.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of my dilemma when choosing a name for this was that we'll have tests like these that always run against momento local, and the standard integration tests that could run with it. I wasn't sure whether we should separate those. We can solve that problem in the future, I suppose.
| markers = [ | ||
| "local: tests that require Momento Local", | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
|
|
||
|
|
||
| class MomentoRpcMethod(Enum): | ||
| GET = ("_GetRequest", "get") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I didn't know the enum values could be tuples.
|
|
||
|
|
||
| class TestFixedCountRetryStrategyAsync: | ||
| @pytest.mark.local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL
Add a middleware that lets tests control Momento Local with metadata and can collect metrics about retries.
Add sync and async tests for the fixed count retry strategy.
Update the aio retry interceptor to stop if from ignoring the result of a future in some cases.
Add a make target for local tests.
Add a github action to run the local tests.