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

Add test for retry token expiration #2118

Open
gretchenfrage opened this issue Dec 24, 2024 · 4 comments · May be fixed by #2149
Open

Add test for retry token expiration #2118

gretchenfrage opened this issue Dec 24, 2024 · 4 comments · May be fixed by #2149
Labels
good first issue Good for newcomers

Comments

@gretchenfrage
Copy link
Collaborator

Now that #2089 is merged, we should add a test for Retry token expiration. One could start by modifying this test:

fn stateless_retry() {

@gretchenfrage gretchenfrage added the good first issue Good for newcomers label Dec 24, 2024
@yohachiSuga
Copy link

Hi @gretchenfrage, could I take a look at this issue and start working on it?

I plan to write a test to cover the following branch by using TimeSource trait.

if retry.issued + server_config.retry_token_lifetime < server_config.time_source.now() {

@gretchenfrage
Copy link
Collaborator Author

Hi @gretchenfrage, could I take a look at this issue and start working on it?

I plan to write a test to cover the following branch by using TimeSource trait.

quinn/quinn-proto/src/token.rs

Line 60 in f5b1ec7
if retry.issued + server_config.retry_token_lifetime < server_config.time_source.now() {

Go ahead!

In a PR I have in-progress, #1912, I create a FakeTimeSource implementation of TimeSource and use it to test the expiration of a different kind of token that that PR also introduces. You're welcome to copy/paste that FakeTimeSource code into a branch you create if you think that's the best way to create your test.

@yohachiSuga
Copy link

Thank you for the information!
It seems that new test cases may need to use the IncomingBehaviour enum. I noticed that #1912 modifies this enum and TestEndpoint. To avoid conflicts, I’ll wait for your PR to be merged before submitting mine.

@gretchenfrage
Copy link
Collaborator Author

@yohachiSuga #1912 has just been merged :)

@yohachiSuga yohachiSuga linked a pull request Jan 31, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants