From b7be0a1fedfeec26e4b7d35357157d5a9209f3ee Mon Sep 17 00:00:00 2001 From: Max Nikulin Date: Tue, 6 Aug 2024 16:10:12 +0700 Subject: Fix copy on write for amount_t::in_place_roundto - Copy amount quantity before rounding. - Fix `amount_t::roundto(int)`. - Transform `balance/testRound` into `balance/testRoundto`. It was a mix of `round` and `roundto` operation with unclear purpose and was relying on `in_place_roundto` behavior modifying all copies. There is no unit tests for balance `round` and `unround` any more. Closes #2362 --- src/amount.cc | 9 +-------- src/amount.h | 2 +- test/unit/t_amount.cc | 5 ++--- test/unit/t_balance.cc | 22 ++++++---------------- 4 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/amount.cc b/src/amount.cc index 604519df..da8cd6fc 100644 --- a/src/amount.cc +++ b/src/amount.cc @@ -685,14 +685,7 @@ void amount_t::in_place_roundto(int places) if (! quantity) throw_(amount_error, _("Cannot round an uninitialized amount")); - // - // _dup() call (see in_place_ceiling and in_place_floor) - // leads to 2 failures in the balance/testRound test, - // however in its absence this method affects copies of amount_t instances. - // Remove expected_failures from amount/testRound - // and update balance/testRound - // when uncommenting the following line. - // _dup(); + _dup(); mpz_t& scale(temp); if (places) diff --git a/src/amount.h b/src/amount.h index 7abaa1c9..5638abc9 100644 --- a/src/amount.h +++ b/src/amount.h @@ -347,7 +347,7 @@ public: amount_t roundto(int places) const { amount_t temp(*this); - temp.in_place_round(); + temp.in_place_roundto(places); return temp; } void in_place_roundto(int places); diff --git a/test/unit/t_amount.cc b/test/unit/t_amount.cc index 28bc56e9..f0fdd73e 100644 --- a/test/unit/t_amount.cc +++ b/test/unit/t_amount.cc @@ -1163,16 +1163,15 @@ BOOST_AUTO_TEST_CASE(testCommodityCeiling) BOOST_CHECK(x2.valid()); } -BOOST_AUTO_TEST_CASE(testRound, * boost::unit_test::expected_failures(1)) +BOOST_AUTO_TEST_CASE(testRound) { amount_t a1("$ 123.123"); amount_t a2(a1); a2.in_place_roundto(2); - // Fails due to missing _dup() call in amount_t::in_place_roundto(int). - // BOOST_CHECK_EQUAL(amount_t("$ 123.123"), a1); // Should it be "$ 123.12"? BOOST_CHECK_EQUAL(amount_t("$ 123.120"), a2); + BOOST_CHECK_EQUAL(amount_t("$ 123.120"), a1.roundto(2)); } #ifndef NOT_FOR_PYTHON diff --git a/test/unit/t_balance.cc b/test/unit/t_balance.cc index 066f066e..b730f93b 100644 --- a/test/unit/t_balance.cc +++ b/test/unit/t_balance.cc @@ -429,7 +429,7 @@ BOOST_AUTO_TEST_CASE(testFloor) BOOST_CHECK(b4.valid()); } -BOOST_AUTO_TEST_CASE(testRound) +BOOST_AUTO_TEST_CASE(testRoundto) { amount_t a1("0.00"); amount_t a2("$ 123.123"); @@ -451,9 +451,6 @@ BOOST_AUTO_TEST_CASE(testRound) b2 += a5; b2 += a6; - // - // This block modifies b1 and b2, likely it is a bug, - // but otherwise some assertions fails. a1.in_place_roundto(2); a2.in_place_roundto(2); a3.in_place_roundto(2); @@ -468,18 +465,11 @@ BOOST_AUTO_TEST_CASE(testRound) b4 += a5; b4 += a6; - // After fix of #2362 rounded() and in_place_round() - // likely should be replaced with roundto(2) and - // in_place_roundto(2). - // It looks like rounded() and in_place_round() - // need some other tests, perhaps with unround(). - BOOST_CHECK_EQUAL(b0.rounded(), b0); - BOOST_CHECK_EQUAL(b2.rounded(), b4); - // Relies on b1 modified by amount_t::in_place_roundto(int). - BOOST_CHECK_EQUAL(b1.rounded(), b4); - - b1.in_place_round(); - // Relies on b1 modified by amount_t::in_place_roundto(int). + BOOST_CHECK_EQUAL(b0.roundto(2), b0); + BOOST_CHECK_EQUAL(b2.roundto(2), b4); + BOOST_CHECK_EQUAL(b1.roundto(2), b4); + + b1.in_place_roundto(2); BOOST_CHECK_EQUAL(b1, b3); BOOST_CHECK(b0.valid()); -- cgit v1.2.3