Skip to content

Commit

Permalink
cksum/hashsum: improve the tests and wording
Browse files Browse the repository at this point in the history
  • Loading branch information
sylvestre committed Jun 3, 2024
1 parent 773d8cf commit 1cbb4d9
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 74 deletions.
16 changes: 7 additions & 9 deletions src/uu/cksum/src/cksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::path::Path;
use uucore::checksum::{
calculate_blake2b_length, detect_algo, digest_reader, perform_checksum_validation,
ChecksumError, ALGORITHM_OPTIONS_BLAKE2B, ALGORITHM_OPTIONS_BSD, ALGORITHM_OPTIONS_CRC,
ALGORITHM_OPTIONS_SYSV, SUPPORTED_ALGO,
ALGORITHM_OPTIONS_SYSV, SUPPORTED_ALGORITHMS,
};
use uucore::{
encoding,
Expand Down Expand Up @@ -278,15 +278,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
};

if check {
let text_flag: bool = matches.get_flag(options::TEXT);
let binary_flag: bool = matches.get_flag(options::BINARY);
let text_flag = matches.get_flag(options::TEXT);
let binary_flag = matches.get_flag(options::BINARY);
let strict = matches.get_flag(options::STRICT);
let status = matches.get_flag(options::STATUS);
let warn = matches.get_flag(options::WARN);
let ignore_missing: bool = matches.get_flag(options::IGNORE_MISSING);
let quiet: bool = matches.get_flag(options::QUIET);
let ignore_missing = matches.get_flag(options::IGNORE_MISSING);
let quiet = matches.get_flag(options::QUIET);

if (binary_flag || text_flag) && check {
if binary_flag || text_flag {
return Err(ChecksumError::BinaryTextConflict.into());

Check warning on line 290 in src/uu/cksum/src/cksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cksum/src/cksum.rs#L290

Added line #L290 was not covered by tests
}
// Determine the appropriate algorithm option to pass
Expand Down Expand Up @@ -364,7 +364,7 @@ pub fn uu_app() -> Command {
.short('a')
.help("select the digest type to use. See DIGEST below")
.value_name("ALGORITHM")
.value_parser(SUPPORTED_ALGO),
.value_parser(SUPPORTED_ALGORITHMS),
)
.arg(
Arg::new(options::UNTAGGED)
Expand Down Expand Up @@ -445,14 +445,12 @@ pub fn uu_app() -> Command {
)
.arg(
Arg::new(options::STATUS)
.short('s')
.long("status")
.help("don't output anything, status code shows success")
.action(ArgAction::SetTrue),
)
.arg(
Arg::new(options::QUIET)
.short('q')
.long(options::QUIET)
.help("don't print OK for each successfully verified file")
.action(ArgAction::SetTrue),
Expand Down
27 changes: 7 additions & 20 deletions src/uu/hashsum/src/hashsum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use uucore::checksum::escape_filename;
use uucore::checksum::perform_checksum_validation;
use uucore::checksum::ChecksumError;
use uucore::checksum::HashAlgorithm;
use uucore::checksum::ALGORITHM_OPTIONS_BLAKE2B;
use uucore::error::{FromIo, UResult};
use uucore::sum::{Digest, Sha3_224, Sha3_256, Sha3_384, Sha3_512, Shake128, Shake256};
use uucore::{format_usage, help_about, help_usage};
Expand Down Expand Up @@ -150,7 +149,7 @@ fn create_algorithm_from_flags(matches: &ArgMatches) -> UResult<HashAlgorithm> {
}

if alg.is_none() {
return Err(ChecksumError::NeedAlgoToHash.into());
return Err(ChecksumError::NeedAlgorithmToHash.into());

Check warning on line 152 in src/uu/hashsum/src/hashsum.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/hashsum/src/hashsum.rs#L152

Added line #L152 was not covered by tests
}

Ok(alg.unwrap())

Check warning on line 155 in src/uu/hashsum/src/hashsum.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/hashsum/src/hashsum.rs#L155

Added line #L155 was not covered by tests
Expand Down Expand Up @@ -190,13 +189,7 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> {
};

let length = match input_length {
Some(length) => {
if binary_name == ALGORITHM_OPTIONS_BLAKE2B || binary_name == "b2sum" {
calculate_blake2b_length(*length)?
} else {
return Err(ChecksumError::LengthOnlyForBlake2b.into());
}
}
Some(length) => calculate_blake2b_length(*length)?,
None => None,
};

Expand All @@ -223,7 +216,7 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> {
let quiet = matches.get_flag("quiet") || status;
//let strict = matches.get_flag("strict");
let warn = matches.get_flag("warn") && !status;
let zero: bool = matches.get_flag("zero");
let zero = matches.get_flag("zero");
let ignore_missing = matches.get_flag("ignore-missing");

if ignore_missing && !check {
Expand All @@ -248,19 +241,13 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> {
};

if check {
let text_flag: bool = matches.get_flag("text");
let binary_flag: bool = matches.get_flag("binary");
let text_flag = matches.get_flag("text");
let binary_flag = matches.get_flag("binary");
let strict = matches.get_flag("strict");

if (binary_flag || text_flag) && check {
if binary_flag || text_flag {
return Err(ChecksumError::BinaryTextConflict.into());

Check warning on line 249 in src/uu/hashsum/src/hashsum.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/hashsum/src/hashsum.rs#L249

Added line #L249 was not covered by tests
}
// Determine the appropriate algorithm option to pass
let algo_option = if algo.name.is_empty() {
None
} else {
Some(algo.name)
};

// Execute the checksum validation based on the presence of files or the use of stdin
// Determine the source of input: a list of files or stdin.
Expand All @@ -278,7 +265,7 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> {
binary_flag,
ignore_missing,
quiet,
algo_option,
Some(algo.name),
Some(algo.bits),
);
}
Expand Down
25 changes: 12 additions & 13 deletions src/uucore/src/lib/features/checksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub const ALGORITHM_OPTIONS_SM3: &str = "sm3";
pub const ALGORITHM_OPTIONS_SHAKE128: &str = "shake128";
pub const ALGORITHM_OPTIONS_SHAKE256: &str = "shake256";

pub const SUPPORTED_ALGO: [&str; 15] = [
pub const SUPPORTED_ALGORITHMS: [&str; 15] = [
ALGORITHM_OPTIONS_SYSV,
ALGORITHM_OPTIONS_BSD,
ALGORITHM_OPTIONS_CRC,
Expand Down Expand Up @@ -82,7 +82,7 @@ pub enum ChecksumError {
BinaryTextConflict,
AlgorithmNotSupportedWithCheck,
CombineMultipleAlgorithms,
NeedAlgoToHash,
NeedAlgorithmToHash,
NoProperlyFormattedChecksumLinesFound(String),

Check warning on line 86 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L86

Added line #L86 was not covered by tests
}

Expand Down Expand Up @@ -129,7 +129,7 @@ impl Display for ChecksumError {
Self::CombineMultipleAlgorithms => {
write!(f, "You cannot combine multiple hash algorithms!")

Check warning on line 130 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L130

Added line #L130 was not covered by tests
}
Self::NeedAlgoToHash => write!(
Self::NeedAlgorithmToHash => write!(

Check warning on line 132 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L132

Added line #L132 was not covered by tests
f,
"Needs an algorithm to hash with.\nUse --help for more information."
),
Expand Down Expand Up @@ -302,7 +302,7 @@ pub fn detect_algo(algo: &str, length: Option<usize>) -> UResult<HashAlgorithm>
})
}

Check warning on line 303 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L303

Added line #L303 was not covered by tests
//ALGORITHM_OPTIONS_SHA3 | "sha3" => (
alg if alg.starts_with("sha3") => create_sha3(length),
_ if algo.starts_with("sha3") => create_sha3(length),

_ => Err(ChecksumError::UnknownAlgorithm.into()),

Check warning on line 307 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L307

Added line #L307 was not covered by tests
}
Expand Down Expand Up @@ -407,10 +407,9 @@ where

let reader = BufReader::new(file);
let lines: Vec<String> = reader.lines().collect::<Result<_, _>>()?;
let (chosen_regex, algo_based_format) =
let (chosen_regex, is_algo_based_format) =
determine_regex(filename_input, input_is_stdin, &lines)?;

// Process each line
for (i, line) in lines.iter().enumerate() {
if let Some(caps) = chosen_regex.captures(line) {
properly_formatted = true;
Expand All @@ -427,7 +426,7 @@ where
let expected_checksum = caps.name("checksum").unwrap().as_str();

// If the algo_name is provided, we use it, otherwise we try to detect it
let (algo_name, length) = if algo_based_format {
let (algo_name, length) = if is_algo_based_format {
// When the algo-based format is matched, extract details from regex captures
let algorithm = caps.name("algo").map_or("", |m| m.as_str()).to_lowercase();

Expand All @@ -440,7 +439,7 @@ where
continue;
}

if !SUPPORTED_ALGO.contains(&algorithm.as_str()) {
if !SUPPORTED_ALGORITHMS.contains(&algorithm.as_str()) {
// Not supported algo, leave early
properly_formatted = false;

Check warning on line 444 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L444

Added line #L444 was not covered by tests
continue;
Expand All @@ -452,7 +451,7 @@ where
Some(Some(bits_value / 8))
} else {
properly_formatted = false;
None // Return None to signal a parsing or divisibility issue
None // Return None to signal a divisibility issue

Check warning on line 454 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L453-L454

Added lines #L453 - L454 were not covered by tests
}
});

Expand All @@ -464,14 +463,15 @@ where

(algorithm, bits.unwrap())
} else if let Some(a) = algo_name_input {
// When a specific algorithm name is input, use it and default bits to None
// When a specific algorithm name is input, use it and use the provided bits
(a.to_lowercase(), length_input)
} else {
// Default case if no algorithm is specified and non-algo based format is matched
(String::new(), None)
};

if algo_based_format && algo_name_input.map_or(false, |input| algo_name != input) {
if is_algo_based_format && algo_name_input.map_or(false, |input| algo_name != input)
{
bad_format += 1;

Check warning on line 475 in src/uucore/src/lib/features/checksum.rs

View check run for this annotation

Codecov / codecov/patch

src/uucore/src/lib/features/checksum.rs#L475

Added line #L475 was not covered by tests
continue;
}
Expand Down Expand Up @@ -579,7 +579,6 @@ where
util_name(),
filename_input.maybe_quote(),
);
//skip_summary = true;
set_exit_code(1);
}

Expand Down Expand Up @@ -640,7 +639,7 @@ pub fn digest_reader<T: Read>(
}
}

/// Calculates the length of the digest for the given algorithm.
/// Calculates the length of the digest.
pub fn calculate_blake2b_length(length: usize) -> UResult<Option<usize>> {
match length {
0 => Ok(None),
Expand Down
9 changes: 2 additions & 7 deletions tests/by-util/test_cksum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,15 +1192,10 @@ fn test_check_directory_error() {
"f",
"BLAKE2b (d) = 786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce\n"
);
let err_msg: &str;
#[cfg(not(windows))]
{
err_msg = "cksum: d: Is a directory\n";
}
let err_msg = "cksum: d: Is a directory\n";
#[cfg(windows)]
{
err_msg = "cksum: d: Permission denied\n";
}
let err_msg = "cksum: d: Permission denied\n";
ucmd.arg("--check")
.arg(at.subdir.join("f"))
.fails()
Expand Down
27 changes: 2 additions & 25 deletions tests/by-util/test_hashsum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,15 +837,10 @@ fn test_check_directory_error() {

at.mkdir("d");
at.write("in.md5", "d41d8cd98f00b204e9800998ecf8427f d\n");
let err_msg: &str;
#[cfg(not(windows))]
{
err_msg = "md5sum: d: Is a directory\n";
}
let err_msg = "md5sum: d: Is a directory\n";
#[cfg(windows)]
{
err_msg = "md5sum: d: Permission denied\n";
}
let err_msg = "md5sum: d: Permission denied\n";
scene
.ccmd("md5sum")
.arg("--check")
Expand Down Expand Up @@ -895,21 +890,3 @@ fn test_star_to_start() {
.succeeds()
.stdout_only("f: OK\n");
}

#[test]
fn test_b2sum_128() {
let scene = TestScenario::new(util_name!());
let at = &scene.fixtures;

at.touch("f");
at.write("in.b2sum", "786a02f742015903c6c6fd852552d272912f4740e15847618a86e217f71f5419d25e1031afee585313896444934eb04b903a685b1448b755d56f701afe9be2ce /dev/null\n");
scene
.ccmd("b2sum")
.arg("--check")
.arg("-l")
.arg("128")
.arg(at.subdir.join("in.b2sum"))
.succeeds()
.stderr_is("")
.stdout_is("/dev/null: OK\n");
}

0 comments on commit 1cbb4d9

Please sign in to comment.