Skip to content

Commit

Permalink
Add improved unit tests and missing impls
Browse files Browse the repository at this point in the history 
The `Neg` test doesn't pass but the others do. I added the neg test so
it doesn't get forgotten.
  • Loading branch information
@tcharding
tcharding committed Feb 18, 2025
1 parent 7f21b76 commit 7f5f8c8
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 151 deletions.
37 changes: 34 additions & 3 deletions units/src/amount/result.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ crate::internal_macros::impl_op_for_references! {

fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error() }
}

impl ops::Add<NumOpResult<Amount>> for Amount {
type Output = NumOpResult<Amount>;

Expand All @@ -141,7 +140,6 @@ crate::internal_macros::impl_op_for_references! {

fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error() }
}

impl ops::Sub<NumOpResult<Amount>> for Amount {
type Output = NumOpResult<Amount>;

Expand All @@ -158,23 +156,39 @@ crate::internal_macros::impl_op_for_references! {

fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
}
impl ops::Mul<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;

fn mul(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs * rhs) }
}

impl ops::Div<u64> for Amount {
type Output = NumOpResult<Amount>;

fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
}
impl ops::Div<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;

fn div(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
}

impl ops::Rem<u64> for Amount {
type Output = NumOpResult<Amount>;

fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
}
impl ops::Rem<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;

fn rem(self, modulus: u64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
}

impl ops::Add<SignedAmount> for SignedAmount {
type Output = NumOpResult<SignedAmount>;

fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error() }
}

impl ops::Add<NumOpResult<SignedAmount>> for SignedAmount {
type Output = NumOpResult<SignedAmount>;

Expand Down Expand Up @@ -202,16 +216,33 @@ crate::internal_macros::impl_op_for_references! {

fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
}
impl ops::Mul<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;

fn mul(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs * rhs) }
}

impl ops::Div<i64> for SignedAmount {
type Output = NumOpResult<SignedAmount>;

fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
}
impl ops::Div<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;

fn div(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
}

impl ops::Rem<i64> for SignedAmount {
type Output = NumOpResult<SignedAmount>;

fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
}
impl ops::Rem<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;

fn rem(self, modulus: i64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
}

impl<T> ops::Add<NumOpResult<T>> for NumOpResult<T>
where
Expand Down
248 changes: 100 additions & 148 deletions units/src/amount/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1196,170 +1196,125 @@ fn check_const() {
assert_eq!(Amount::MAX_MONEY.to_sat() as i64, SignedAmount::MAX_MONEY.to_sat());
}

// Verify we have implemented all combinations of ops for `Amount` and `SignedAmount`.
// It's easier to read this test that check the code.
// Sanity check than stdlib supports the set of reference combinations for the ops we want.
#[test]
#[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
fn amount_tyes_all_ops() {
// Sanity check than stdlib supports the set of reference combinations for the ops we want.
{
let x = 127;
fn sanity_all_ops() {
let x = 127;

let _ = x + x;
let _ = &x + x;
let _ = x + &x;
let _ = &x + &x;
let _ = x + x;
let _ = &x + x;
let _ = x + &x;
let _ = &x + &x;

let _ = x - x;
let _ = &x - x;
let _ = x - &x;
let _ = &x - &x;
let _ = x - x;
let _ = &x - x;
let _ = x - &x;
let _ = &x - &x;

let _ = -x;
}
let _ = -x;
}

// Verify we have implemented all combinations of ops for the amount types and `NumOpResult` type.
// It's easier to read this test than check the code.
#[test]
#[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
fn num_op_result_ops() {
let sat = Amount::from_sat(1);
let ssat = SignedAmount::from_sat(1);

// Add
let _ = sat + sat;
let _ = &sat + sat;
let _ = sat + &sat;
let _ = &sat + &sat;

// let _ = ssat + sat;
// let _ = &ssat + sat;
// let _ = ssat + &sat;
// let _ = &ssat + &sat;

// let _ = sat + ssat;
// let _ = &sat + ssat;
// let _ = sat + &ssat;
// let _ = &sat + &ssat;

let _ = ssat + ssat;
let _ = &ssat + ssat;
let _ = ssat + &ssat;
let _ = &ssat + &ssat;

// Sub
let _ = sat - sat;
let _ = &sat - sat;
let _ = sat - &sat;
let _ = &sat - &sat;

// let _ = ssat - sat;
// let _ = &ssat - sat;
// let _ = ssat - &sat;
// let _ = &ssat - &sat;

// let _ = sat - ssat;
// let _ = &sat - ssat;
// let _ = sat - &ssat;
// let _ = &sat - &ssat;

let _ = ssat - ssat;
let _ = &ssat - ssat;
let _ = ssat - &ssat;
let _ = &ssat - &ssat;

// let _ = sat * sat; // Intentionally not supported.

// Mul
let _ = sat * 3;
let _ = sat * &3;
let _ = &sat * 3;
let _ = &sat * &3;

let _ = ssat * 3_i64; // Explicit type for the benefit of the reader.
let _ = ssat * &3;
let _ = &ssat * 3;
let _ = &ssat * &3;

// Div
let _ = sat / 3;
let _ = &sat / 3;
let _ = sat / &3;
let _ = &sat / &3;

let _ = ssat / 3_i64; // Explicit type for the benefit of the reader.
let _ = &ssat / 3;
let _ = ssat / &3;
let _ = &ssat / &3;

// Rem
let _ = sat % 3;
let _ = &sat % 3;
let _ = sat % &3;
let _ = &sat % &3;

let _ = ssat % 3;
let _ = &ssat % 3;
let _ = ssat % &3;
let _ = &ssat % &3;

// FIXME: Do we want to support this?
// let _ = sat / sat;
//
// "How many times does this amount go into that amount?" - seems
// like a reasonable question to ask.

// FIXME: Do we want to support these?
// let _ = -sat;
// let _ = -ssat;
// Explicit type as sanity check.
let res: NumOpResult<Amount> = sat + sat;
let sres: NumOpResult<SignedAmount> = ssat + ssat;

macro_rules! check_op {
($(let _ = $lhs:ident $op:tt $rhs:ident);* $(;)?) => {
$(
let _ = $lhs $op $rhs;
let _ = &$lhs $op $rhs;
let _ = $lhs $op &$rhs;
let _ = &$lhs $op &$rhs;
)*
}
}

// We do not currently support division involving `NumOpResult` and an amount type.
check_op! {
// Operations where RHS is the result of another operation.
let _ = sat + res;
let _ = sat - res;
// let _ = sat / res;
let _ = ssat + sres;
let _ = ssat - sres;
// let _ = ssat / sres;

// Operations where LHS is the result of another operation.
let _ = res + sat;
let _ = res - sat;
// let _ = res / sat;
let _ = sres + ssat;
let _ = sres - ssat;
// let _ = sres / ssat;

// Operations that where both sides are the result of another operation.
let _ = res + res;
let _ = res - res;
// let _ = res / res;
let _ = sres + sres;
let _ = sres - sres;
// let _ = sres / sres;
};
}

// FIXME: Should we support this sort of thing?
// It will be a lot more code for possibly not that much benefit.
// Verify we have implemented all combinations of ops for the `NumOpResult` type and an integer.
// It's easier to read this test than check the code.
#[test]
fn can_ops_on_amount_and_signed_amount() {
// let res: NumOpResult<SignedAmount> = sat + ssat;
#[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
fn num_op_result_ops_integer() {
let sat = Amount::from_sat(1);
let ssat = SignedAmount::from_sat(1);

// Explicit type as sanity check.
let res: NumOpResult<Amount> = sat + sat;
let sres: NumOpResult<SignedAmount> = ssat + ssat;

macro_rules! check_op {
($(let _ = $lhs:ident $op:tt $rhs:literal);* $(;)?) => {
$(
let _ = $lhs $op $rhs;
let _ = &$lhs $op $rhs;
let _ = $lhs $op &$rhs;
let _ = &$lhs $op &$rhs;
)*
}
}
check_op! {
// Operations on a `NumOpResult` and integer.
let _ = res * 3_u64; // Explicit type for the benefit of the reader.
let _ = res / 3;
let _ = res % 3;

let _ = sres * 3_i64; // Explicit type for the benefit of the reader.
let _ = sres / 3;
let _ = sres % 3;
};
}

// Verify we have implemented all combinations of ops for the `NumOpResult` type.
// It's easier to read this test that check the code.
// Verify we have implemented all `Neg` for the amount types.
#[test]
#[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
fn amount_op_result_all_ops() {
fn amount_op_result_neg() {
let sat = Amount::from_sat(1);
// let ssat = SignedAmount::from_sat(1);
let ssat = SignedAmount::from_sat(1);

// Explicit type as sanity check.
let res: NumOpResult<Amount> = sat + sat;
// let sres: NumOpResult<SignedAmount> = ssat + ssat;

// Operations that where RHS is the result of another operation.
let _ = sat + res;
let _ = &sat + res;
// let _ = sat + &res;
// let _ = &sat + &res;

let _ = sat - res;
let _ = &sat - res;
// let _ = sat - &res;
// let _ = &sat - &res;

// Operations that where LHS is the result of another operation.
let _ = res + sat;
// let _ = &res + sat;
let _ = res + &sat;
// let _ = &res + &sat;

let _ = res - sat;
// let _ = &res - sat;
let _ = res - &sat;
// let _ = &res - &sat;

// Operations that where both sides are the result of another operation.
let _ = res + res;
// let _ = &res + res;
// let _ = res + &res;
// let _ = &res + &res;

let _ = res - res;
// let _ = &res - res;
// let _ = res - &res;
// let _ = &res - &res;
let sres: NumOpResult<SignedAmount> = ssat + ssat;

let _ = -sat;
let _ = -ssat;

let _ = -res;
let _ = -sres;
}

// Verify we have implemented all `Sum` for the `NumOpResult` type.
Expand All @@ -1373,7 +1328,4 @@ fn amount_op_result_sum() {
let _ = amounts.iter().sum::<NumOpResult<Amount>>();
let _ = amount_refs.iter().copied().sum::<NumOpResult<Amount>>();
let _ = amount_refs.into_iter().sum::<NumOpResult<Amount>>();

// FIXME: Should we support this? I don't think so (Tobin).
// let _ = amount_refs.iter().sum::<NumOpResult<&Amount>>();
}

0 comments on commit 7f5f8c8

Please sign in to comment.