emelie/zola
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

Treat 304 as valid, add mock tests, fix mock issue (#900)

Browse source 
* Treat 304 (Not Modified) requests as valid.

* Add tests for 301-to-200 links, 301-to-404 links, and 500 links.
This helps to test redirections and the previously-added
response.status() checking for non-success status codes in check_url().

* Make names for HTTP mock paths unique, to avoid weird behavior. It
seems like mocks with the same path can potentially bleed between
tests, so you may end up with an unexpected response which causes the
test to sometimes pass and sometimes fail.

* Fix Clippy warnings about String::from(format!()).
This commit is contained in:
Sam Ford 2019-12-24 06:29:50 -05:00 committed by Vincent Prouillet
parent 11f7a6d114
commit 6b5768fd76
2 changed files with 97 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

119
components/link_checker/src/lib.rs
View file

@ -23,7 +23,7 @@ impl LinkResult {
} }
if let Some(c) = self.code { if let Some(c) = self.code {
return c.is_success(); return c.is_success() || c == StatusCode::NOT_MODIFIED;
} }
true true
@ -72,31 +72,19 @@ pub fn check_url(url: &str, config: &LinkChecker) -> LinkResult {
} }
} }
Ok(response) => { Ok(response) => {
if response.status().is_success() { if response.status().is_success() || response.status() == StatusCode::NOT_MODIFIED {
LinkResult { code: Some(response.status()), error: None } LinkResult { code: Some(response.status()), error: None }
} else { } else {
let error_string = if response.status().is_informational() { let error_string = if response.status().is_informational() {
String::from(format!( format!("Informational status code ({}) received", response.status())
"Informational status code ({}) received",
response.status()
))
} else if response.status().is_redirection() { } else if response.status().is_redirection() {
String::from(format!( format!("Redirection status code ({}) received", response.status())
"Redirection status code ({}) received",
response.status()
))
} else if response.status().is_client_error() { } else if response.status().is_client_error() {
String::from(format!( format!("Client error status code ({}) received", response.status())
"Client error status code ({}) received",
response.status()
))
} else if response.status().is_server_error() { } else if response.status().is_server_error() {
String::from(format!( format!("Server error status code ({}) received", response.status())
"Server error status code ({}) received",
response.status()
))
} else { } else {
String::from("Non-success status code received") format!("Non-success status code ({}) received", response.status())
}; };
LinkResult { code: None, error: Some(error_string) } LinkResult { code: None, error: Some(error_string) }
@ -142,11 +130,16 @@ mod tests {
use super::{check_page_for_anchor, check_url, has_anchor, LinkChecker, LINKS}; use super::{check_page_for_anchor, check_url, has_anchor, LinkChecker, LINKS};
use mockito::mock; use mockito::mock;
// NOTE: HTTP mock paths below are randomly generated to avoid name
// collisions. Mocks with the same path can sometimes bleed between tests
// and cause them to randomly pass/fail. Please make sure to use unique
// paths when adding or modifying tests that use Mockito.
#[test] #[test]
fn can_validate_ok_links() { fn can_validate_ok_links() {
let url = format!("{}{}", mockito::server_url(), "/test"); let url = format!("{}{}", mockito::server_url(), "/ekbtwxfhjw");
let _m = mock("GET", "/test") let _m = mock("GET", "/ekbtwxfhjw")
.with_header("content-type", "text/html") .with_header("Content-Type", "text/html")
.with_body(format!( .with_body(format!(
r#"<!DOCTYPE html> r#"<!DOCTYPE html>
<html> <html>
@ -168,8 +161,43 @@ mod tests {
} }
#[test] #[test]
fn can_fail_unresolved_links() { fn can_follow_301_links() {
let res = check_url("https://google.comys", &LinkChecker::default()); let _m1 = mock("GET", "/c7qrtrv3zz")
.with_status(301)
.with_header("Content-Type", "text/plain")
.with_header("Location", format!("{}/rbs5avjs8e", mockito::server_url()).as_str())
.with_body("Redirecting...")
.create();
let _m2 = mock("GET", "/rbs5avjs8e")
.with_header("Content-Type", "text/plain")
.with_body("Test")
.create();
let url = format!("{}{}", mockito::server_url(), "/c7qrtrv3zz");
let res = check_url(&url, &LinkChecker::default());
assert!(res.is_valid());
assert!(res.code.is_some());
assert!(res.error.is_none());
}
#[test]
fn can_fail_301_to_404_links() {
let _m1 = mock("GET", "/cav9vibhsc")
.with_status(301)
.with_header("Content-Type", "text/plain")
.with_header("Location", format!("{}/72zmfg4smd", mockito::server_url()).as_str())
.with_body("Redirecting...")
.create();
let _m2 = mock("GET", "/72zmfg4smd")
.with_status(404)
.with_header("Content-Type", "text/plain")
.with_body("Not Found")
.create();
let url = format!("{}{}", mockito::server_url(), "/cav9vibhsc");
let res = check_url(&url, &LinkChecker::default());
assert_eq!(res.is_valid(), false); assert_eq!(res.is_valid(), false);
assert!(res.code.is_none()); assert!(res.code.is_none());
assert!(res.error.is_some()); assert!(res.error.is_some());
@ -177,19 +205,42 @@ mod tests {
#[test] #[test]
fn can_fail_404_links() { fn can_fail_404_links() {
let _m = mock("GET", "/404") let _m = mock("GET", "/nlhab9c1vc")
.with_status(404) .with_status(404)
.with_header("content-type", "text/plain") .with_header("Content-Type", "text/plain")
.with_body("Not Found") .with_body("Not Found")
.create(); .create();
let url = format!("{}{}", mockito::server_url(), "/404"); let url = format!("{}{}", mockito::server_url(), "/nlhab9c1vc");
let res = check_url(&url, &LinkChecker::default()); let res = check_url(&url, &LinkChecker::default());
assert_eq!(res.is_valid(), false); assert_eq!(res.is_valid(), false);
assert!(res.code.is_none()); assert!(res.code.is_none());
assert!(res.error.is_some()); assert!(res.error.is_some());
} }
#[test]
fn can_fail_500_links() {
let _m = mock("GET", "/qdbrssazes")
.with_status(500)
.with_header("Content-Type", "text/plain")
.with_body("Internal Server Error")
.create();
let url = format!("{}{}", mockito::server_url(), "/qdbrssazes");
let res = check_url(&url, &LinkChecker::default());
assert_eq!(res.is_valid(), false);
assert!(res.code.is_none());
assert!(res.error.is_some());
}
#[test]
fn can_fail_unresolved_links() {
let res = check_url("https://t6l5cn9lpm.lxizfnzckd", &LinkChecker::default());
assert_eq!(res.is_valid(), false);
assert!(res.code.is_none());
assert!(res.error.is_some());
}
#[test] #[test]
fn can_validate_anchors() { fn can_validate_anchors() {
let url = "https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.collect"; let url = "https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.collect";
@ -255,8 +306,8 @@ mod tests {
let ignore_url = format!("{}{}", mockito::server_url(), "/ignore/"); let ignore_url = format!("{}{}", mockito::server_url(), "/ignore/");
let config = LinkChecker { skip_prefixes: vec![], skip_anchor_prefixes: vec![ignore_url] }; let config = LinkChecker { skip_prefixes: vec![], skip_anchor_prefixes: vec![ignore_url] };
let _m1 = mock("GET", "/ignore/test") let _m1 = mock("GET", "/ignore/i30hobj1cy")
.with_header("content-type", "text/html") .with_header("Content-Type", "text/html")
.with_body( .with_body(
r#"<!DOCTYPE html> r#"<!DOCTYPE html>
<html> <html>
@ -272,11 +323,11 @@ mod tests {
.create(); .create();
// anchor check is ignored because the url matches the prefix // anchor check is ignored because the url matches the prefix
let ignore = format!("{}{}", mockito::server_url(), "/ignore/test#nonexistent"); let ignore = format!("{}{}", mockito::server_url(), "/ignore/i30hobj1cy#nonexistent");
assert!(check_url(&ignore, &config).is_valid()); assert!(check_url(&ignore, &config).is_valid());
let _m2 = mock("GET", "/test") let _m2 = mock("GET", "/guvqcqwmth")
.with_header("content-type", "text/html") .with_header("Content-Type", "text/html")
.with_body( .with_body(
r#"<!DOCTYPE html> r#"<!DOCTYPE html>
<html> <html>
@ -292,10 +343,10 @@ mod tests {
.create(); .create();
// other anchors are checked // other anchors are checked
let existent = format!("{}{}", mockito::server_url(), "/test#existent"); let existent = format!("{}{}", mockito::server_url(), "/guvqcqwmth#existent");
assert!(check_url(&existent, &config).is_valid()); assert!(check_url(&existent, &config).is_valid());
let nonexistent = format!("{}{}", mockito::server_url(), "/test#nonexistent"); let nonexistent = format!("{}{}", mockito::server_url(), "/guvqcqwmth#nonexistent");
assert_eq!(check_url(&nonexistent, &config).is_valid(), false); assert_eq!(check_url(&nonexistent, &config).is_valid(), false);
} }
} }

19
components/templates/src/global_fns/load_data.rs
View file

@ -325,6 +325,11 @@ mod tests {
use serde_json::json; use serde_json::json;
use tera::{to_value, Function}; use tera::{to_value, Function};
// NOTE: HTTP mock paths below are randomly generated to avoid name
// collisions. Mocks with the same path can sometimes bleed between tests
// and cause them to randomly pass/fail. Please make sure to use unique
// paths when adding or modifying tests that use Mockito.
fn get_test_file(filename: &str) -> PathBuf { fn get_test_file(filename: &str) -> PathBuf {
let test_files = PathBuf::from("../utils/test-files").canonicalize().unwrap(); let test_files = PathBuf::from("../utils/test-files").canonicalize().unwrap();
return test_files.join(filename); return test_files.join(filename);
@ -366,14 +371,14 @@ mod tests {
#[test] #[test]
fn calculates_cache_key_for_url() { fn calculates_cache_key_for_url() {
let _m = mock("GET", "/test") let _m = mock("GET", "/kr1zdgbm4y")
.with_header("content-type", "text/plain") .with_header("content-type", "text/plain")
.with_body("Test") .with_body("Test")
.create(); .create();
let url = format!("{}{}", mockito::server_url(), "/test"); let url = format!("{}{}", mockito::server_url(), "/kr1zdgbm4y");
let cache_key = DataSource::Url(url.parse().unwrap()).get_cache_key(&OutputFormat::Plain); let cache_key = DataSource::Url(url.parse().unwrap()).get_cache_key(&OutputFormat::Plain);
assert_eq!(cache_key, 12502656262443320092); assert_eq!(cache_key, 425638486551656875);
} }
#[test] #[test]
@ -396,7 +401,7 @@ mod tests {
#[test] #[test]
fn can_load_remote_data() { fn can_load_remote_data() {
let _m = mock("GET", "/json") let _m = mock("GET", "/zpydpkjj67")
.with_header("content-type", "application/json") .with_header("content-type", "application/json")
.with_body( .with_body(
r#"{ r#"{
@ -408,7 +413,7 @@ mod tests {
) )
.create(); .create();
let url = format!("{}{}", mockito::server_url(), "/json"); let url = format!("{}{}", mockito::server_url(), "/zpydpkjj67");
let static_fn = LoadData::new(PathBuf::new()); let static_fn = LoadData::new(PathBuf::new());
let mut args = HashMap::new(); let mut args = HashMap::new();
args.insert("url".to_string(), to_value(&url).unwrap()); args.insert("url".to_string(), to_value(&url).unwrap());
@ -419,13 +424,13 @@ mod tests {
#[test] #[test]
fn fails_when_request_404s() { fn fails_when_request_404s() {
let _m = mock("GET", "/404") let _m = mock("GET", "/aazeow0kog")
.with_status(404) .with_status(404)
.with_header("content-type", "text/plain") .with_header("content-type", "text/plain")
.with_body("Not Found") .with_body("Not Found")
.create(); .create();
let url = format!("{}{}", mockito::server_url(), "/404"); let url = format!("{}{}", mockito::server_url(), "/aazeow0kog");
let static_fn = LoadData::new(PathBuf::new()); let static_fn = LoadData::new(PathBuf::new());
let mut args = HashMap::new(); let mut args = HashMap::new();
args.insert("url".to_string(), to_value(&url).unwrap()); args.insert("url".to_string(), to_value(&url).unwrap());