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.

Sign up for GitHub

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

Jump to bottom

Tests for sentences/utils.rs #88

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Tests for sentences/utils.rs #88

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4f7ff3
add tests
gflow33 Apr 18, 2023
44ce869
format code
gflow33 Apr 18, 2023
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
73 changes: 73 additions & 0 deletions src/sentences/utils.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good approach would be to check the actual error that is returned that just expect a panic. A panic might be occurring for something different that you are expecting in a specific test case so try to assert a specific error on each test.

This will make sure that your test is failing of the right reasons that you expect it to.

Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,79 @@ mod tests {
assert_eq!(time.nanosecond(), 500_000_000);
}

#[test]
#[should_panic]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which of the 2 tests here should panic? Maybe you can split them if they both need to panic as this will only check the first one

fn test_parse_hms_fail_1() {
use chrono::Timelike;
let (_, time) = parse_hms("247090,").unwrap();
assert_eq!(time.hour(), 24);
assert_eq!(time.minute(), 70);
assert_eq!(time.second(), 90);
assert_eq!(time.nanosecond(), 0);
let (_, time) = parse_hms("247090.8,").unwrap();
assert_eq!(time.hour(), 24);
assert_eq!(time.minute(), 70);
assert_eq!(time.second(), 90);
assert_eq!(time.nanosecond(), 800_000_000);
}

#[test]
#[should_panic]
fn test_parse_hms_hr_fail() {
use chrono::Timelike;
let (_, time) = parse_hms("244550,").unwrap();
assert_eq!(time.hour(), 24);
assert_eq!(time.minute(), 45);
assert_eq!(time.second(), 50);
assert_eq!(time.nanosecond(), 0);
}

#[test]
#[should_panic]
fn test_parse_hms_min_fail() {
use chrono::Timelike;
let (_, time) = parse_hms("237050,").unwrap();
assert_eq!(time.hour(), 23);
assert_eq!(time.minute(), 70);
assert_eq!(time.second(), 50);
assert_eq!(time.nanosecond(), 0);
Comment on lines +234 to +238
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If calling parse_hms(...).unwrap() will panic the rest of the assertions will not be executed.
It would be best to assert the expected error instead of unwraping in this case.
This way you can be sure that if minutes is out of bound, then you get an minutes error not something else.

Right now, you cannot be sure if the minutes are the culprit of the error.

}
#[test]
#[should_panic]
fn test_parse_hms_sec_fail() {
use chrono::Timelike;
let (_, time) = parse_hms("234590,").unwrap();
assert_eq!(time.hour(), 23);
assert_eq!(time.minute(), 45);
assert_eq!(time.second(), 90);
assert_eq!(time.nanosecond(), 0);
}

#[test]
fn test_parse_hms_fail_2() {
let invalid_time_hr = "254550,";
let result_hr = parse_hms(invalid_time_hr);
let expected_err_hr = nom::Err::Error(nom::error::Error::new(
"254550,",
nom::error::ErrorKind::MapRes,
));
assert_eq!(result_hr, Err(expected_err_hr));
let invalid_time_min = "237050,";
let result_min = parse_hms(invalid_time_min);
let expected_err_min = nom::Err::Error(nom::error::Error::new(
"237050,",
nom::error::ErrorKind::MapRes,
));
assert_eq!(result_min, Err(expected_err_min));
let invalid_time_sec = "234570,";
let result_sec = parse_hms(invalid_time_sec);
let expected_err_sec = nom::Err::Error(nom::error::Error::new(
"234570,",
nom::error::ErrorKind::MapRes,
));
assert_eq!(result_sec, Err(expected_err_sec));
}

#[test]
fn test_parse_date() {
let (_, date) = parse_date("180283").unwrap();
Expand Down