Skip to content

Commit

Permalink
Rename fail_map to error_map for improved clarity in response sta…
Browse files Browse the repository at this point in the history
…tistics (#1560)

Fixes #1446
  • Loading branch information
mre authored Nov 8, 2024
1 parent f4a35ff commit 9dc4217
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 28 deletions.
2 changes: 1 addition & 1 deletion assets/screencast.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion lychee-bin/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ fn show_progress(

fn get_failed_urls(stats: &mut ResponseStats) -> Vec<(InputSource, Url)> {
stats
.fail_map
.error_map
.iter()
.flat_map(|(source, set)| {
set.iter()
Expand Down
14 changes: 7 additions & 7 deletions lychee-bin/src/formatters/stats/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ impl Display for CompactResponseStats {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let stats = &self.stats;

if !stats.fail_map.is_empty() {
let input = if stats.fail_map.len() == 1 {
if !stats.error_map.is_empty() {
let input = if stats.error_map.len() == 1 {
"input"
} else {
"inputs"
Expand All @@ -31,13 +31,13 @@ impl Display for CompactResponseStats {
f,
BOLD_PINK,
"Issues found in {} {input}. Find details below.\n\n",
stats.fail_map.len()
stats.error_map.len()
)?;
}

let response_formatter = get_response_formatter(&self.mode);

for (source, responses) in &stats.fail_map {
for (source, responses) in &stats.error_map {
color!(f, BOLD_YELLOW, "[{}]:\n", source)?;
for response in responses {
writeln!(
Expand Down Expand Up @@ -145,9 +145,9 @@ mod tests {
status: Status::Ok(StatusCode::INTERNAL_SERVER_ERROR),
};

let mut fail_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let mut error_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let source = InputSource::RemoteUrl(Box::new(Url::parse("https://example.com").unwrap()));
fail_map.insert(source, HashSet::from_iter(vec![err1, err2]));
error_map.insert(source, HashSet::from_iter(vec![err1, err2]));

let stats = ResponseStats {
total: 1,
Expand All @@ -157,7 +157,7 @@ mod tests {
excludes: 0,
timeouts: 0,
duration_secs: 0,
fail_map,
error_map,
suggestion_map: HashMap::default(),
unsupported: 0,
redirects: 0,
Expand Down
8 changes: 4 additions & 4 deletions lychee-bin/src/formatters/stats/detailed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Display for DetailedResponseStats {

let response_formatter = get_response_formatter(&self.mode);

for (source, responses) in &stats.fail_map {
for (source, responses) in &stats.error_map {
// Using leading newlines over trailing ones (e.g. `writeln!`)
// lets us avoid extra newlines without any additional logic.
write!(f, "\n\nErrors in {source}")?;
Expand Down Expand Up @@ -115,9 +115,9 @@ mod tests {
status: Status::Ok(StatusCode::INTERNAL_SERVER_ERROR),
};

let mut fail_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let mut error_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let source = InputSource::RemoteUrl(Box::new(Url::parse("https://example.com").unwrap()));
fail_map.insert(source, HashSet::from_iter(vec![err1, err2]));
error_map.insert(source, HashSet::from_iter(vec![err1, err2]));

let stats = ResponseStats {
total: 2,
Expand All @@ -132,7 +132,7 @@ mod tests {
cached: 0,
suggestion_map: HashMap::default(),
success_map: HashMap::default(),
fail_map,
error_map,
excluded_map: HashMap::default(),
detailed_stats: true,
};
Expand Down
2 changes: 1 addition & 1 deletion lychee-bin/src/formatters/stats/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl Display for MarkdownResponseStats {
writeln!(f)?;
writeln!(f, "{}", stats_table(&self.0))?;

write_stats_per_input(f, "Errors", &stats.fail_map, |response| {
write_stats_per_input(f, "Errors", &stats.error_map, |response| {
markdown_response(response).map_err(|_e| fmt::Error)
})?;

Expand Down
2 changes: 1 addition & 1 deletion lychee-bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ async fn run(opts: &LycheeOptions) -> Result<i32> {
let (stats, cache, exit_code) = commands::check(params).await?;

let github_issues = stats
.fail_map
.error_map
.values()
.flatten()
.any(|body| body.uri.domain() == Some("github.com"));
Expand Down
18 changes: 9 additions & 9 deletions lychee-bin/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use serde::Serialize;
///
/// This struct contains various counters for the responses received during a
/// run. It also contains maps to store the responses for each status (success,
/// fail, excluded, etc.) and the sources of the responses.
/// error, excluded, etc.) and the sources of the responses.
///
/// The `detailed_stats` field is used to enable or disable the storage of the
/// responses in the maps for successful and excluded responses. If it's set to
Expand Down Expand Up @@ -39,7 +39,7 @@ pub(crate) struct ResponseStats {
/// Map to store successful responses (if `detailed_stats` is enabled)
pub(crate) success_map: HashMap<InputSource, HashSet<ResponseBody>>,
/// Map to store failed responses (if `detailed_stats` is enabled)
pub(crate) fail_map: HashMap<InputSource, HashSet<ResponseBody>>,
pub(crate) error_map: HashMap<InputSource, HashSet<ResponseBody>>,
/// Replacement suggestions for failed responses (if `--suggest` is enabled)
pub(crate) suggestion_map: HashMap<InputSource, HashSet<Suggestion>>,
/// Map to store excluded responses (if `detailed_stats` is enabled)
Expand Down Expand Up @@ -91,7 +91,7 @@ impl ResponseStats {
let status = response.status();
let source = response.source().clone();
let status_map_entry = match status {
_ if status.is_error() => self.fail_map.entry(source).or_default(),
_ if status.is_error() => self.error_map.entry(source).or_default(),
Status::Ok(_) if self.detailed_stats => self.success_map.entry(source).or_default(),
Status::Excluded if self.detailed_stats => self.excluded_map.entry(source).or_default(),
_ => return,
Expand Down Expand Up @@ -174,9 +174,9 @@ mod tests {
stats.add(dummy_ok());

let response = dummy_error();
let expected_fail_map: HashMap<InputSource, HashSet<ResponseBody>> =
let expected_error_map: HashMap<InputSource, HashSet<ResponseBody>> =
HashMap::from_iter([(response.source().clone(), HashSet::from_iter([response.1]))]);
assert_eq!(stats.fail_map, expected_fail_map);
assert_eq!(stats.error_map, expected_error_map);

assert!(stats.success_map.is_empty());
}
Expand All @@ -185,20 +185,20 @@ mod tests {
async fn test_detailed_stats() {
let mut stats = ResponseStats::extended();
assert!(stats.success_map.is_empty());
assert!(stats.fail_map.is_empty());
assert!(stats.error_map.is_empty());
assert!(stats.excluded_map.is_empty());

stats.add(dummy_error());
stats.add(dummy_excluded());
stats.add(dummy_ok());

let mut expected_fail_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let mut expected_error_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let response = dummy_error();
let entry = expected_fail_map
let entry = expected_error_map
.entry(response.source().clone())
.or_default();
entry.insert(response.1);
assert_eq!(stats.fail_map, expected_fail_map);
assert_eq!(stats.error_map, expected_error_map);

let mut expected_success_map: HashMap<InputSource, HashSet<ResponseBody>> = HashMap::new();
let response = dummy_ok();
Expand Down
62 changes: 59 additions & 3 deletions lychee-bin/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mod cli {
errors: usize,
cached: usize,
success_map: HashMap<InputSource, HashSet<ResponseBody>>,
fail_map: HashMap<InputSource, HashSet<ResponseBody>>,
error_map: HashMap<InputSource, HashSet<ResponseBody>>,
suggestion_map: HashMap<InputSource, HashSet<ResponseBody>>,
excluded_map: HashMap<InputSource, HashSet<ResponseBody>>,
}
Expand Down Expand Up @@ -156,6 +156,61 @@ mod cli {
Ok(())
}

/// Test JSON output format
#[tokio::test]
async fn test_json_output() -> Result<()> {
// Server that returns a bunch of 200 OK responses
let mock_server_ok = mock_server!(StatusCode::OK);
let mut cmd = main_command();
cmd.arg("--format")
.arg("json")
.arg("-vv")
.arg("--no-progress")
.arg("-")
.write_stdin(mock_server_ok.uri())
.assert()
.success();
let output = cmd.output().unwrap();
let output_json = serde_json::from_slice::<Value>(&output.stdout)?;

// Check that the output is valid JSON
assert!(output_json.is_object());
// Check that the output contains the expected keys
assert!(output_json.get("detailed_stats").is_some());
assert!(output_json.get("success_map").is_some());
assert!(output_json.get("error_map").is_some());
assert!(output_json.get("excluded_map").is_some());

// Check the success map
let success_map = output_json["success_map"].as_object().unwrap();
assert_eq!(success_map.len(), 1);

// Get the actual URL from the mock server for comparison
let mock_url = mock_server_ok.uri();

// Create the expected success map structure
let expected_success_map = serde_json::json!({
"stdin": [
{
"status": {
"code": 200,
"text": "200 OK"
},
"url": format!("{mock_url}/"),
}
]
});

// Compare the actual success map with the expected one
assert_eq!(
success_map,
expected_success_map.as_object().unwrap(),
"Success map doesn't match expected structure"
);

Ok(())
}

/// JSON-formatted output should always be valid JSON.
/// Additional hints and error messages should be printed to `stderr`.
/// See https://github.com/lycheeverse/lychee/issues/1355
Expand Down Expand Up @@ -195,9 +250,10 @@ mod cli {
// Check that the output is valid JSON
assert!(serde_json::from_slice::<Value>(&output.stdout).is_ok());

// Parse site error status from the fail_map
// Parse site error status from the error_map
let output_json = serde_json::from_slice::<Value>(&output.stdout).unwrap();
let site_error_status = &output_json["fail_map"][&test_path.to_str().unwrap()][0]["status"];
let site_error_status =
&output_json["error_map"][&test_path.to_str().unwrap()][0]["status"];

assert_eq!(
"error sending request for url (https://expired.badssl.com/) Maybe a certificate error?",
Expand Down
2 changes: 1 addition & 1 deletion lychee-lib/src/types/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{InputSource, Status, Uri};

/// Response type returned by lychee after checking a URI
//
// Body is public to allow inserting into stats maps (fail_map, success_map,
// Body is public to allow inserting into stats maps (error_map, success_map,
// etc.) without `Clone`, because the inner `ErrorKind` in `response.status` is
// not `Clone`. Use `body()` to access the body in the rest of the code.
//
Expand Down

0 comments on commit 9dc4217

Please sign in to comment.