-
Notifications
You must be signed in to change notification settings - Fork 110
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
Switch artichoke-backend to use spinoso-time tzrs feature #1956
Conversation
10acbd4
to
2770c74
Compare
Hi @b-n I've seen this but haven't had time to sit down and give a proper review yet. I took a glance and things look like they came together nicely. I hope to get to this sometime this week. |
Heads up, despite the crate being named The |
@lopopolo I'll pick this one up again shortly, sorry for taking a while to get back around to it! |
Also, do you have a preference on tabled vs. non tabled tests? (below are the same tests) Table tests: #[test]
fn subsec_millis_tabled() {
let mut interp = interpreter();
let table: HashMap<&[u8], (i64, u32)> = HashMap::from([
(b"-1001".as_slice(), (-2, 999_000_000)),
(b"-1000".as_slice(), (-2, 0)),
(b"-999".as_slice(), (-1, 1_000_000)),
(b"-1".as_slice(), (-1, 999_000_000)),
(b"0".as_slice(), (0, 0)),
(b"999".as_slice(), (0, 999_000_000)),
(b"1000".as_slice(), (1, 0)),
(b"1001".as_slice(), (1, 1_000_000))
]);
let unit = b":milliseconds";
for (input, expectation) in table {
let result = subsec(&mut interp, (Some(input), Some(unit))).unwrap();
assert_eq!(
result.to_tuple(),
expectation,
"Expected TryConvertMut<(Some({}), Some({})), Result<Subsec>>, to return {} secs, {} nanos",
input.as_bstr(),
unit.as_bstr(),
expectation.0,
expectation.1
);
}
} Not so tabled: #[test]
fn subsec_millis() {
let mut interp = interpreter();
let result = subsec(&mut interp, (Some(b"-1001"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (-2, -999_000_000));
let result = subsec(&mut interp, (Some(b"-1000"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (-2, 0));
let result = subsec(&mut interp, (Some(b"-999"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (-1, 1_000_000));
let result = subsec(&mut interp, (Some(b"-1"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (-1, 999_000_000));
let result = subsec(&mut interp, (Some(b"0"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (0, 0));
let result = subsec(&mut interp, (Some(b"999"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (0, 999_000_000));
let result = subsec(&mut interp, (Some(b"1000"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (1, 0));
let result = subsec(&mut interp, (Some(b"1001"), Some(b":milliseconds"))).unwrap();
assert_eq!(result.to_tuple(), (1, 1_000_000));
} |
I've been leaning toward table driven tests lately, especially for things like parsers, and have been liking how clean things look. I haven't been using artichoke/scolapasta-int-parse/src/error.rs Lines 348 to 362 in 995b8ca
|
Ruby 3.x supports a lot more than what is expected form 2.7.x. This is the base for the changes to support ruby 3.x changes at least
subsec could be a Fixnum or a Float. This logic is now handled in the subsec module.
Co-authored-by: Ryan Lopopolo <rjl@hyperbo.la>
Co-authored-by: Ryan Lopopolo <rjl@hyperbo.la>
eca5eb7
to
acd634a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments resolved and fixups filed as tickets.
Will merge this when the build is green.
#1927 Added enough to hopefully make tzrs feature safe for replacement of the chrono feature in arthicoke-backend. This PR achieves to do exactly that
Compatibility
The previous version of
Time#at
supported only the syntax thatmruby
did. This PR introduces all options available as at MRI 3.1.2.Time#succ
now uses the same logic asTime#+
which wasn't implemented before.Time#+
is now implemented, and follows MRITime#-
changes: