Address a couple bugs

Author: nekevss

src/builtins/core/duration.rs

@@ -10,7 +10,7 @@ use crate::{
     parsers::{FormattableDateDuration, FormattableDuration, FormattableTimeDuration, Precision},
     primitive::FiniteF64,
     provider::TimeZoneProvider,
-    temporal_assert, Sign, TemporalError, TemporalResult, TemporalUnwrap,
+    temporal_assert, Sign, TemporalError, TemporalResult, TemporalUnwrap, NS_PER_DAY,
 };
 use alloc::format;
 use alloc::string::String;
@@ -658,7 +658,6 @@ impl Duration {
                 let internal = NormalizedDurationRecord::from_duration_with_24_hour_days(self)?;
                 // 31. If smallestUnit is day, then
                 let internal = if resolved_options.smallest_unit == Unit::Day {
-                    // TODO: TEST
                     // a. Let fractionalDays be TotalTimeDuration(internalDuration.[[Time]], day).
                     // b. Let days be RoundNumberToIncrement(fractionalDays, roundingIncrement, roundingMode).
                     let days = internal
@@ -670,7 +669,7 @@ impl Duration {
                     // c. Let dateDuration be ? CreateDateDurationRecord(0, 0, 0, days).
                     let date = DateDuration::new(0, 0, 0, days)?;
                     // d. Set internalDuration to CombineDateAndTimeDuration(dateDuration, 0).
-                    NormalizedDurationRecord::new(date, norm)?
+                    NormalizedDurationRecord::new(date, NormalizedTimeDuration::default())?
                 // 32. Else,
                 } else {
                     // TODO: update round / round_inner methods
@@ -769,8 +768,9 @@ impl Duration {
                     return Err(TemporalError::range());
                 }
                 // c. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
+                let internal = NormalizedDurationRecord::from_duration_with_24_hour_days(self)?;
                 // d. Let total be TotalTimeDuration(internalDuration.[[Time]], unit).
-                let total = self.time.to_normalized().total(unit)?;
+                let total = internal.normalized_time_duration().total(unit)?;
                 Ok(total)
             }
         }
@@ -867,6 +867,7 @@ pub fn duration_to_formattable(
 // TODO: Update, optimize, and fix the below. is_valid_duration should probably be generic over a T.
 
 const TWO_POWER_FIFTY_THREE: i128 = 9_007_199_254_740_992;
+const MAX_SAFE_NS_PRECISION: i128 = TWO_POWER_FIFTY_THREE * 1_000_000_000;
 
 // NOTE: Can FiniteF64 optimize the duration_validation
 /// Utility function to check whether the `Duration` fields are valid.
@@ -933,15 +934,16 @@ pub(crate) fn is_valid_duration(
     // in C++ with an implementation of core::remquo() with sufficient bits in the quotient.
     // String manipulation will also give an exact result, since the multiplication is by a power of 10.
     // Seconds part
-    let normalized_seconds =
-        (days as i128 * 86_400) + (hours as i128) * 3600 + minutes as i128 * 60 + seconds as i128;
+    // TODO: Fix the below parts after clarification around behavior.
+    let normalized_nanoseconds =
+        (days as i128 * NS_PER_DAY as i128) + (hours as i128) * 3_600_000_000_000 + minutes as i128 * 60_000_000_000 + seconds as i128 * 1_000_000_000;
     // Subseconds part
     let normalized_subseconds_parts =
-        (milliseconds as i128 / 1_000) + (microseconds / 1_000_000) + (nanoseconds / 1_000_000_000);
+        (milliseconds as i128 * 1_000_000) + (microseconds * 1_000) + nanoseconds;
 
-    let normalized_seconds = normalized_seconds + normalized_subseconds_parts;
+    let normalized_seconds = normalized_nanoseconds + normalized_subseconds_parts;
     // 8. If abs(normalizedSeconds) ≥ 2**53, return false.
-    if normalized_seconds.abs() >= TWO_POWER_FIFTY_THREE {
+    if normalized_seconds.abs() >= MAX_SAFE_NS_PRECISION  {
         return false;
     }
 

src/builtins/core/duration/tests.rs

@@ -1,6 +1,6 @@
 use core::str::FromStr;
 
-use crate::{options::ToStringRoundingOptions, parsers::Precision, partial::PartialDuration};
+use crate::{options::{RoundingOptions, ToStringRoundingOptions, Unit}, parsers::Precision, partial::PartialDuration, provider::NeverProvider};
 
 use super::Duration;
 
@@ -184,3 +184,53 @@ fn duration_from_str() {
     assert_eq!(duration.microseconds(), 999);
     assert_eq!(duration.nanoseconds(), 940);
 }
+
+#[test]
+fn duration_max() {
+    let cases = [
+        (Duration::new(0, 0, 0, 104249991374, 7, 36, 31, 999, 999, 999).unwrap(), "max days", 9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, 2501999792983, 36, 31, 999, 999, 999).unwrap(), "max hours", 9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, 0, 150119987579016, 31, 999, 999, 999).unwrap(), "max minutes", 9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, 0, 0, 9007199254740991, 999, 999, 999).unwrap(), "max seconds", 9007199254740991.999999999),
+        (Duration::new(0, 0, 0, -104249991374, -7, -36, -31, -999, -999, -999).unwrap(), "min days", -9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, -2501999792983, -36, -31, -999, -999, -999).unwrap(), "min hours", -9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, 0, -150119987579016, -31, -999, -999, -999).unwrap(), "min minutes", -9007199254740991.999999999),
+        (Duration::new(0, 0, 0, 0, 0, 0, -9007199254740991, -999, -999, -999).unwrap(), "min seconds", -9007199254740991.999999999),
+      ];
+
+    for (duration, description, result) in cases {
+        assert_eq!(duration.total_with_provider(Unit::Second, None, &NeverProvider).unwrap().0, result, "{description}");
+    }
+}
+
+#[test]
+fn duration_round_negative() {
+    let duration = Duration::new(0, 0, 0, 0, -60, 0, 0, 0, 0, 0).unwrap();
+    let result = duration.round_with_provider(RoundingOptions {
+        smallest_unit: Some(Unit::Day),
+        ..Default::default()
+    }, None, &NeverProvider).unwrap();
+    assert_eq!(result.days(), -3);
+
+}
+
+/*
+TODO: Uncomment
+
+The below test should fail, but currently doesn't. This has to do with weird
+floating point math in IsValidDuration Step 6-8 that defers to C++ std::remquo
+
+Needs further clarification.
+
+#[test]
+fn duration_round_out_of_range_norm_conversion() {
+    const MAX_SAFE_INT: i64 = 9_007_199_254_740_991;
+    let duration = Duration::new(0, 0, 0, 0, 0, 0, MAX_SAFE_INT, 0, 0, 999_999_999).unwrap();
+    let err = duration.round_with_provider( RoundingOptions {
+        largest_unit: Some(Unit::Nanosecond),
+        increment: Some(RoundingIncrement::ONE),
+        ..Default::default()
+    }, None, &NeverProvider);
+    assert!(err.is_err())
+}
+*/