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

Remove the 'static requirement on TestRequest::with_path #225

Merged
merged 1 commit into from
Jun 26, 2022

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Apr 6, 2022

With 'static, one can only use static string slices, which makes it impossible to test with urls generated at runtime, such as ones
generated by a fuzzer. We can drop the 'static requirement to enable this use case.

This change is backwards-compatible, because it only relaxes the requirements on with_path, and although the type of the field changed, that field is not exposed.

This does not incur an extra copy either. The copy was already there, it only got moved out of from and into with_path now.

@ruuda ruuda changed the title Remove the 'static requirement on TestRequest::path Remove the 'static requirement on TestRequest::with_path Apr 6, 2022
With 'static, one can only use static string slices, which makes it
impossible to test with urls generated at runtime, such as ones
generated by a fuzzer.

This change is backwards-compatible, because it only relaxes the
requirements on `with_path`, and although the type of the field changed,
that field is not exposed.

This doesn't incur an extra copy either. The copy was already there,
it only got moved out of `into` and into `with_path` now.
@bradfier
Copy link
Member

Looks like a bit of a bug in Github Actions here where it's requiring an old version of a matrix to pass.

I'll merge anyway, sorry it took a while I was waiting for a green check to appear!

@bradfier bradfier merged commit f0fce7e into tiny-http:master Jun 26, 2022
@ruuda ruuda deleted the test-no-static-path branch June 27, 2022 15:57
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.

None yet

2 participants