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

Sort compact/detailed/markdown error output by file path #1622

Merged
merged 5 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lychee-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ tokio = { version = "1.42.0", features = ["full"] }
tokio-stream = "0.1.17"
toml = "0.8.19"
url = "2.5.4"
human-sort = "0.2.2"

[dev-dependencies]
assert_cmd = "2.0.16"
Expand Down
13 changes: 10 additions & 3 deletions lychee-bin/src/formatters/stats/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Display for CompactResponseStats {

let response_formatter = get_response_formatter(&self.mode);

for (source, responses) in &stats.error_map {
for (source, responses) in super::sort_stat_map(&stats.error_map) {
color!(f, BOLD_YELLOW, "[{}]:\n", source)?;
for response in responses {
writeln!(
Expand All @@ -47,9 +47,16 @@ impl Display for CompactResponseStats {
)?;
}

if let Some(suggestions) = &stats.suggestion_map.get(source) {
if let Some(suggestions) = stats.suggestion_map.get(source) {
// Sort suggestions
let mut sorted_suggestions: Vec<_> = suggestions.iter().collect();
sorted_suggestions.sort_by(|a, b| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
});

writeln!(f, "\n\u{2139} Suggestions")?;
for suggestion in *suggestions {
for suggestion in sorted_suggestions {
writeln!(f, "{suggestion}")?;
}
}
Expand Down
19 changes: 13 additions & 6 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.error_map {
for (source, responses) in super::sort_stat_map(&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 All @@ -60,12 +60,19 @@ impl Display for DetailedResponseStats {
"\n{}",
response_formatter.format_detailed_response(response)
)?;
}

if let Some(suggestions) = &stats.suggestion_map.get(source) {
writeln!(f, "\nSuggestions in {source}")?;
for suggestion in *suggestions {
writeln!(f, "{suggestion}")?;
}
if let Some(suggestions) = stats.suggestion_map.get(source) {
// Sort suggestions
let mut sorted_suggestions: Vec<_> = suggestions.iter().collect();
sorted_suggestions.sort_by(|a, b| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
});

writeln!(f, "\nSuggestions in {source}")?;
for suggestion in sorted_suggestions {
writeln!(f, "{suggestion}")?;
}
}
}
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 @@ -127,7 +127,7 @@ where
{
if !&map.is_empty() {
writeln!(f, "\n## {name} per input")?;
for (source, responses) in map {
for (source, responses) in super::sort_stat_map(map) {
writeln!(f, "\n### {name} in {source}\n")?;
for response in responses {
writeln!(f, "{}", write_stat(response)?)?;
Expand Down
100 changes: 100 additions & 0 deletions lychee-bin/src/formatters/stats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,110 @@ pub(crate) use json::Json;
pub(crate) use markdown::Markdown;
pub(crate) use raw::Raw;

use std::{
collections::{HashMap, HashSet},
fmt::Display,
};

use crate::stats::ResponseStats;
use anyhow::Result;
use lychee_lib::InputSource;

pub(crate) trait StatsFormatter {
/// Format the stats of all responses and write them to stdout
fn format(&self, stats: ResponseStats) -> Result<Option<String>>;
}

/// Convert a `ResponseStats` `HashMap` to a sorted Vec of key-value pairs
/// The returned keys and values are both sorted in natural, case-insensitive order
fn sort_stat_map<T>(stat_map: &HashMap<InputSource, HashSet<T>>) -> Vec<(&InputSource, Vec<&T>)>
where
T: Display,
{
let mut entries: Vec<_> = stat_map
.iter()
.map(|(source, responses)| {
let mut sorted_responses: Vec<&T> = responses.iter().collect();
sorted_responses.sort_by(|a, b| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
});

(source, sorted_responses)
})
.collect();

entries.sort_by(|(a, _), (b, _)| {
let (a, b) = (a.to_string().to_lowercase(), b.to_string().to_lowercase());
human_sort::compare(&a, &b)
});

entries
}

#[cfg(test)]
mod tests {
use super::*;

use lychee_lib::{ErrorKind, Response, Status, Uri};
use url::Url;

fn make_test_url(url: &str) -> Url {
Url::parse(url).expect("Expected valid Website URI")
}

fn make_test_response(url_str: &str, source: InputSource) -> Response {
let uri = Uri::from(make_test_url(url_str));

Response::new(uri, Status::Error(ErrorKind::InvalidUrlHost), source)
}

#[test]
fn test_sorted_stat_map() {
let mut test_stats = ResponseStats::default();

// Sorted list of test sources
let test_sources = vec![
InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/404"))),
InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/home"))),
InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/page/1"))),
InputSource::RemoteUrl(Box::new(make_test_url("https://example.com/page/10"))),
];

// Sorted list of test responses
let test_response_urls = vec![
"https://example.com/",
"https://github.com/",
"https://itch.io/",
"https://youtube.com/",
];

// Add responses to stats
// Responses are added to a HashMap, so the order is not preserved
for source in &test_sources {
for response in &test_response_urls {
test_stats.add(make_test_response(response, source.clone()));
}
}

// Sort error map and extract the sources
let sorted_errors = sort_stat_map(&test_stats.error_map);
let sorted_sources: Vec<InputSource> = sorted_errors
.iter()
.map(|(source, _)| (*source).clone())
.collect();

// Check that the input sources are sorted
assert_eq!(test_sources, sorted_sources);

// Check that the responses are sorted
for (_, response_bodies) in sorted_errors {
let response_urls: Vec<&str> = response_bodies
.into_iter()
.map(|response| response.uri.as_str())
.collect();

assert_eq!(test_response_urls, response_urls);
}
}
}
47 changes: 47 additions & 0 deletions lychee-bin/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1940,4 +1940,51 @@ mod cli {

Ok(())
}

#[test]
fn test_sorted_error_output() -> Result<()> {
let test_files = ["TEST_GITHUB_404.md", "TEST_INVALID_URLS.html"];

let test_urls = [
"https://httpbin.org/status/404",
"https://httpbin.org/status/500",
"https://httpbin.org/status/502",
];

let cmd = &mut main_command()
.arg("--format")
.arg("compact")
.arg(fixtures_path().join(test_files[1]))
.arg(fixtures_path().join(test_files[0]))
.assert()
.failure()
.code(2);

let output = String::from_utf8_lossy(&cmd.get_output().stdout);
let mut position: usize = 0;

// Check that the input sources are sorted
for file in test_files {
assert!(output.contains(file));

let next_position = output.find(file).unwrap();

assert!(next_position > position);
position = next_position;
}

position = 0;

// Check that the responses are sorted
for url in test_urls {
assert!(output.contains(url));

let next_position = output.find(url).unwrap();

assert!(next_position > position);
position = next_position;
}

Ok(())
}
}