From c95135b0d0168393f4ccca9863ade7efac8b3379 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sun, 24 Nov 2024 18:52:51 -0500 Subject: [PATCH] fix: Parse relative months and years This extends `gix_date::parse::relative::span` to recognize times with "months" and "years", as in "3 months ago" and "2 years ago". Those units are supported by Git. The actual change here to the parsing code is very simple, because it is directly facilitated by #1474. The units "seconds", "minutes", "hours", "days", and "weeks" continue to be recognized (as before). Relative times like "1 year, 2 months ago" remain unrecognized. For background, see 43b6c06 (#498), c5c6bf6, https://github.com/GitoxideLabs/gitoxide/pull/1474#discussion_r1694769988, and https://github.com/GitoxideLabs/gitoxide/issues/1696#issuecomment-2495526654. Note that this specific change does not itself fix issue #1696, which applies to the intepretation of "days" and "weeks", and now also applies in the same way to the interpretation of "months" and "years". (It continues not to apply to the interpretation of "seconds", "minutes", and "hours".) The tests are updated to cover "months" and "years", as well as to exercise a wider range of relative times, including showing which units (e.g. "days") are affected by #1696. A few sub-cases, of those adjusted or added here, test strings from a related test in Git, to allow comparison. But most are not related to cases there. As before, the tests pass on CI, or when the time zone is otherwise UTC, or with local time zones that never have DST adjustments. The sub-cases are reordered to be monotone increasing in the magnitude of the relative intervals tested (and thus monotone decreasing in the associated absolute timestamps), and the original input is kept zipped into the `actual` and `expected` arrays being compared. This way, if the assertion fails (such as due to #1696), it is clear and readable which sub-cases failed and exactly how, as well as what all the sub-cases are and what each sub-case tests. Reordering the sub-cases and preserving the inputs they parse in this way avoids the disadvantages of #1697 while keeping, and expanding on, its benefits. Failure, where it occurs, now looks like: --- STDERR: gix-date::date time::parse::relative::various --- thread 'time::parse::relative::various' panicked at gix-date\tests\time\parse.rs:252:9: assertion failed: `(left == right)`: relative times differ Diff < left / right > : [ ( "5 seconds ago", 2024-11-24T23:51:49Z, ), ( "5 minutes ago", 2024-11-24T23:46:54Z, ), ( "5 hours ago", 2024-11-24T18:51:54Z, ), ( "5 days ago", 2024-11-19T23:51:54Z, ), ( "3 weeks ago", 2024-11-03T23:51:54Z, ), ( "21 days ago", 2024-11-03T23:51:54Z, ), ( "504 hours ago", 2024-11-03T23:51:54Z, ), ( "30240 minutes ago", 2024-11-03T23:51:54Z, ), ( "2 months ago", < 2024-09-24T23:51:54Z, > 2024-09-24T22:51:54Z, ), ( "1460 hours ago", 2024-09-25T03:51:54Z, ), ( "87600 minutes ago", 2024-09-25T03:51:54Z, ), ( "14 weeks ago", < 2024-08-18T23:51:54Z, > 2024-08-18T22:51:54Z, ), ( "98 days ago", < 2024-08-18T23:51:54Z, > 2024-08-18T22:51:54Z, ), ( "2352 hours ago", 2024-08-18T23:51:54Z, ), ( "141120 minutes ago", 2024-08-18T23:51:54Z, ), ( "5 months ago", < 2024-06-24T23:51:54Z, > 2024-06-24T22:51:54Z, ), ( "3650 hours ago", 2024-06-25T21:51:54Z, ), ( "219000 minutes ago", 2024-06-25T21:51:54Z, ), ( "26 weeks ago", < 2024-05-26T23:51:54Z, > 2024-05-26T22:51:54Z, ), ( "182 days ago", < 2024-05-26T23:51:54Z, > 2024-05-26T22:51:54Z, ), ( "4368 hours ago", 2024-05-26T23:51:54Z, ), ( "262080 minutes ago", 2024-05-26T23:51:54Z, ), ( "8 months ago", < 2024-03-24T23:51:54Z, > 2024-03-24T22:51:54Z, ), ( "5840 hours ago", 2024-03-26T15:51:54Z, ), ( "350400 minutes ago", 2024-03-26T15:51:54Z, ), ( "38 weeks ago", 2024-03-03T23:51:54Z, ), ( "266 days ago", 2024-03-03T23:51:54Z, ), ( "6384 hours ago", 2024-03-03T23:51:54Z, ), ( "383040 minutes ago", 2024-03-03T23:51:54Z, ), ( "11 months ago", 2023-12-24T23:51:54Z, ), ( "8030 hours ago", 2023-12-26T09:51:54Z, ), ( "481800 minutes ago", 2023-12-26T09:51:54Z, ), ( "14 months ago", < 2023-09-24T23:51:54Z, > 2023-09-24T22:51:54Z, ), ( "21 months ago", 2023-02-24T23:51:54Z, ), ( "2 years ago", 2022-11-24T23:51:54Z, ), ( "20 years ago", 2004-11-24T23:51:54Z, ), ( "630720000 seconds ago", 2004-11-29T23:51:54Z, ), ] --- gix-date/src/parse.rs | 3 +- gix-date/tests/time/parse.rs | 70 +++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/gix-date/src/parse.rs b/gix-date/src/parse.rs index 376bdc0ba36..cf00b13b383 100644 --- a/gix-date/src/parse.rs +++ b/gix-date/src/parse.rs @@ -153,7 +153,8 @@ mod relative { "hour" => Span::new().try_hours(units), "day" => Span::new().try_days(units), "week" => Span::new().try_weeks(units), - // TODO months & years? YES + "month" => Span::new().try_months(units), + "year" => Span::new().try_years(units), // Ignore values you don't know, assume seconds then (so does git) _ => return None, }; diff --git a/gix-date/tests/time/parse.rs b/gix-date/tests/time/parse.rs index bee1ac4c856..3b4e6e6036f 100644 --- a/gix-date/tests/time/parse.rs +++ b/gix-date/tests/time/parse.rs @@ -181,39 +181,75 @@ mod relative { fn various() { let now = SystemTime::now(); + // For comparison, a few are the same as in: https://github.com/git/git/blob/master/t/t0006-date.sh let cases = [ - ("2 weeks ago", 2.weeks()), + ("5 seconds ago", 5.seconds()), + ("5 minutes ago", 5.minutes()), + ("5 hours ago", 5.hours()), + ("5 days ago", 5.days()), + ("3 weeks ago", 3.weeks()), + ("21 days ago", 21.days()), // 3 weeks + ("504 hours ago", 504.hours()), // 3 weeks + ("30240 minutes ago", 30_240.minutes()), // 3 weeks + ("2 months ago", 2.months()), + ("1460 hours ago", 1460.hours()), // 2 months + ("87600 minutes ago", 87_600.minutes()), // 2 months ("14 weeks ago", 14.weeks()), - ("26 weeks ago", 26.weeks()), - ("38 weeks ago", 38.weeks()), - ("50 weeks ago", 50.weeks()), - ("20160 minutes ago", 20_160.minutes()), // 2 weeks + ("98 days ago", 98.days()), // 14 weeks + ("2352 hours ago", 2352.hours()), // 14 weeks ("141120 minutes ago", 141_120.minutes()), // 14 weeks + ("5 months ago", 5.months()), + ("3650 hours ago", 3650.hours()), // 5 months + ("219000 minutes ago", 219_000.minutes()), // 5 months + ("26 weeks ago", 26.weeks()), + ("182 days ago", 182.days()), // 26 weeks + ("4368 hours ago", 4368.hours()), // 26 weeks ("262080 minutes ago", 262_080.minutes()), // 26 weeks + ("8 months ago", 8.months()), + ("5840 hours ago", 5840.hours()), // 8 months + ("350400 minutes ago", 350_400.minutes()), // 8 months + ("38 weeks ago", 38.weeks()), + ("266 days ago", 266.days()), // 38 weeks + ("6384 hours ago", 6384.hours()), // 38 weeks ("383040 minutes ago", 383_040.minutes()), // 38 weeks - ("504000 minutes ago", 504_000.minutes()), // 50 weeks + ("11 months ago", 11.months()), + ("8030 hours ago", 8030.hours()), // 11 months + ("481800 minutes ago", 481_800.minutes()), // 11 months + ("14 months ago", 14.months()), // "1 year, 2 months ago" not yet supported. + ("21 months ago", 21.months()), // "1 year, 9 months ago" not yet supported. + ("2 years ago", 2.years()), + ("20 years ago", 20.years()), + ("630720000 seconds ago", 630_720_000.seconds()), // 20 years ]; - let times = cases.map(|(input, _)| gix_date::parse(input, Some(now)).unwrap()); - - assert_eq!(times.map(|_| Sign::Plus), times.map(|time| time.sign)); - assert_eq!(times.map(|_| 0), times.map(|time| time.offset)); + let with_times = cases.map(|(input, _)| { + let time = gix_date::parse(input, Some(now)).expect("relative time string should parse to a Time"); + (input, time) + }); + assert_eq!(with_times.map(|_| Sign::Plus), with_times.map(|(_, time)| time.sign)); + assert_eq!(with_times.map(|_| 0), with_times.map(|(_, time)| time.offset)); - let expected = cases.map(|(_, span)| { - Zoned::try_from(now) - .unwrap() + let with_expected = cases.map(|(input, span)| { + let expected = Zoned::try_from(now) + .expect("test needs to convert current time to a timestamp") // account for the loss of precision when creating `Time` with seconds .round( jiff::ZonedRound::new() .smallest(jiff::Unit::Second) .mode(jiff::RoundMode::Trunc), ) - .unwrap() + .expect("test needs to truncate current timestamp to seconds") .saturating_sub(span) - .timestamp() + .timestamp(); + + (input, expected) + }); + let with_actual = with_times.map(|(input, time)| { + let actual = jiff::Timestamp::from_second(time.seconds) + .expect("seconds obtained from a Time should convert to Timestamp"); + (input, actual) }); - let actual = times.map(|time| jiff::Timestamp::from_second(time.seconds).unwrap()); - assert_eq!(actual, expected, "relative times differ"); + assert_eq!(with_actual, with_expected, "relative times differ"); } }