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

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"));
 
-  // <https://github.com/ledger/ledger/issues/2362>
-  // _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).
-  // <https://github.com/ledger/ledger/issues/2362>
   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;
 
-  // <https://github.com/ledger/ledger/issues/2362>
-  // 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());