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

Tests: Recover failed webhooks #365

Merged
merged 1 commit into from
Apr 5, 2022

Conversation

jaymell
Copy link
Contributor

@jaymell jaymell commented Apr 4, 2022

Add tests to ensure proper functioning of recover-failed-webhooks endpoint. This will validate that particular failed messages are retried an expected number of times based upon message creation time and the since time passed to the endpoint.

@jaymell jaymell force-pushed the recover-failed-webhooks branch 3 times, most recently from 7bbb6aa to 6bc0d30 Compare April 4, 2022 21:19
@jaymell jaymell marked this pull request as ready for review April 4, 2022 21:21
@jaymell jaymell closed this Apr 5, 2022
@svix-james svix-james reopened this Apr 5, 2022
@tasn tasn requested a review from svix-daniel April 5, 2022 10:26
@tasn
Copy link
Member

tasn commented Apr 5, 2022

Let's have Daniel take a look, but please fix the title. You are not working on recover failed webhooks, but rather adding tests for it.

@svix-daniel
Copy link
Contributor

Also non-essential, but I usually like to leave assertions and unwraps in the function marked as #[test] so that when they panic, the line number given is actually in the test. A matter of preference though :)

@jaymell jaymell changed the title Recover failed webhooks Tests: Recover failed webhooks Apr 5, 2022
Add tests to ensure that failed webhooks are retried with expected count based on `since` time specified,
and ensure recover requests are rejected if `since` time exceeds age limit
@jaymell
Copy link
Contributor Author

jaymell commented Apr 5, 2022

I refactored based on comments and removed the try_message_attempts functions entirely since I think they were muddling the point.

@svix-daniel svix-daniel merged commit c0643a9 into svix:main Apr 5, 2022
@jaymell jaymell deleted the recover-failed-webhooks branch April 5, 2022 21:02
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.

4 participants