Skip to content

Constantify serve_{dir,file} test file paths and content #541

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nc7s
Copy link

@nc7s nc7s commented Jan 20, 2025

Motivation

Mainly a QoL change for downstream packagers, but might also prove useful for e.g. future refactors.

Currently we have to patch all those file paths to something exsistent, due to test-files/ not being included in crates.io releases, while the Debian Rust Team pulls Rust crate source from there.

Solution

With these constants we could change only them.

Test files are also changed to the same content, so one constant fits all.

Mainly a QoL change for downstream packagers, but might also prove
useful for e.g. future refactors.

Currently we have to patch all those file paths to something exsistent,
due to test-files/ not being included in crates.io releases, while the
Debian Rust Team pulls Rust crate source from there:

https://salsa.debian.org/rust-team/debcargo-conf/-/blob/ee75c0c18a/src/tower-http/debian/patches/missing-testdata

With these constants we could change only them.

Test files are also changed to the same content, so one constant fits
all.
@nc7s nc7s force-pushed the constantify-test-names branch from 6aa0bef to cf9c6be Compare January 20, 2025 16:10
@jplatte
Copy link
Member

jplatte commented May 25, 2025

Hey. I understand you're trying to solve a real issue here, but I'm not a fan of this diff. Why do you run these tests when packaging?

@nc7s
Copy link
Author

nc7s commented May 26, 2025

We frequently patch code in distros, so running tests there make sure our patches don't break things.

The diff - sure could be improved. Now that Debian's in freeze, it's not any urgent.

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.

2 participants