-
-
Notifications
You must be signed in to change notification settings - Fork 293
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
Separate tar archive and tar flags #492
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,3 +51,44 @@ fn archives_are_disabled(tmpdir: TempDir, port: u16) -> Result<(), Error> { | |
|
||
Ok(()) | ||
} | ||
|
||
#[rstest] | ||
deantvv marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you wanna do something cool here, you could use rstest for test parameterization to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, on second thought, I want to more properly clean up these tests anyhow. I'll just do that after merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can try in maybe another PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, if you like, that'd be much appreciated. So what I'm thinking here: Have a single parametrized test in Take a look at: https://docs.rs/rstest/0.7.0/rstest/attr.rstest.html#test-parametrized-cases |
||
fn test_tar_archives(tmpdir: TempDir, port: u16) -> Result<(), Error> { | ||
let mut child = Command::cargo_bin("miniserve")? | ||
.arg(tmpdir.path()) | ||
.arg("-p") | ||
.arg(port.to_string()) | ||
.arg("-g") | ||
.stdout(Stdio::null()) | ||
.spawn()?; | ||
|
||
sleep(Duration::from_secs(1)); | ||
|
||
// Ensure the links to the tar archive exists and tar not exists | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One small thing I just found: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am happy to fix this. But since you already merge this, should I still change this? |
||
let body = reqwest::blocking::get(format!("http://localhost:{}", port).as_str())? | ||
.error_for_status()?; | ||
let parsed = Document::from_read(body)?; | ||
assert!(parsed.find(Text).any(|x| x.text() == "Download .tar.gz")); | ||
assert!(parsed.find(Text).all(|x| x.text() != "Download .tar")); | ||
|
||
// Try to download, only tar_gz should works | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
assert_eq!( | ||
reqwest::blocking::get(format!("http://localhost:{}/?download=tar_gz", port).as_str())? | ||
.status(), | ||
StatusCode::OK | ||
); | ||
assert_eq!( | ||
reqwest::blocking::get(format!("http://localhost:{}/?download=tar", port).as_str())? | ||
.status(), | ||
StatusCode::FORBIDDEN | ||
); | ||
assert_eq!( | ||
reqwest::blocking::get(format!("http://localhost:{}/?download=zip", port).as_str())? | ||
.status(), | ||
StatusCode::FORBIDDEN | ||
); | ||
|
||
child.kill()?; | ||
|
||
Ok(()) | ||
} |
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.
Come to think of it, it probably wasn't proper of me to go with
CompressionMethod
here as it should really have beenArchiveMethod
from the beginning. Oh well, might change this after merging your PR. :)