Skip to content

Commit

Permalink
[FOLD] Review feedback #4:
Browse files Browse the repository at this point in the history
* More test cases for XRPAmount's mulratio function.
* Improve TaggedFee::json to work with non-integral types. Add tests.
  • Loading branch information
ximinez committed Aug 14, 2019
1 parent 2c81a6a commit 5f61eff
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 4 deletions.
4 changes: 3 additions & 1 deletion src/ripple/basics/XRPAmount.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ class XRPAmount
Json::Value
json () const
{
static_assert(std::is_signed_v<value_type>, "Expected XRPAmount to be signed");
static_assert(std::is_signed_v<value_type> &&
std::is_integral_v<value_type>,
"Expected XRPAmount to be a signed integral type");

constexpr auto min = std::numeric_limits<Json::Int>::min();
constexpr auto max = std::numeric_limits<Json::Int>::max();
Expand Down
14 changes: 11 additions & 3 deletions src/ripple/basics/feeunits.h
Original file line number Diff line number Diff line change
Expand Up @@ -276,10 +276,19 @@ class TaggedFee
return fee_;
}

Json::Value
template<class T = value_type>
std::enable_if_t<is_usable_unit_v<TaggedFee> &&
!std::is_integral_v<T>, Json::Value>
json () const
{
return fee_;
}

template<class T = value_type>
std::enable_if_t<is_usable_unit_v<TaggedFee> &&
std::is_integral_v<T>, Json::Value>
json () const
{
static_assert(is_usable_unit_v<TaggedFee>, "Invalid unit type");
using jsontype = std::conditional_t<std::is_signed_v<value_type>,
Json::Int, Json::UInt>;

Expand All @@ -293,7 +302,6 @@ class TaggedFee
return static_cast<jsontype>(fee_);
}


/** Returns the underlying value. Code SHOULD NOT call this
function unless the type has been abstracted away,
e.g. in a templated function.
Expand Down
13 changes: 13 additions & 0 deletions src/test/basics/XRPAmount_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ class XRPAmount_test : public beast::unit_test::suite
BEAST_EXPECT(big == mulRatio (big, maxUInt32, maxUInt32, true));
// rounding mode shouldn't matter as the result is exact
BEAST_EXPECT(big == mulRatio (big, maxUInt32, maxUInt32, false));

// multiply and divide by values that would overflow if done naively,
// and check that it gives the correct answer
big -= 0xf; // Subtract a little so it's divisable by 4
BEAST_EXPECT(mulRatio(big, 3, 4, false) == (big / 4) * 3);
BEAST_EXPECT(mulRatio(big, 3, 4, true) == (big / 4) * 3);
BEAST_EXPECT((big * 3) / 4 != (big / 4) * 3);
}

{
Expand All @@ -151,6 +158,12 @@ class XRPAmount_test : public beast::unit_test::suite
BEAST_EXPECT(big == mulRatio (big, maxUInt32, maxUInt32, true));
// rounding mode shouldn't matter as the result is exact
BEAST_EXPECT(big == mulRatio (big, maxUInt32, maxUInt32, false));

// multiply and divide by values that would overflow if done naively,
// and check that it gives the correct answer
BEAST_EXPECT(mulRatio(big, 3, 4, false) == (big / 4) * 3);
BEAST_EXPECT(mulRatio(big, 3, 4, true) == (big / 4) * 3);
BEAST_EXPECT((big * 3) / 4 != (big / 4) * 3);
}

{
Expand Down
60 changes: 60 additions & 0 deletions src/test/basics/feeunits_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,66 @@ class feeunits_test
feeunit::drop_tag>));
BEAST_EXPECT((std::is_same_v<decltype(drops), XRPAmount>));
}

// Json value functionality
{
FeeUnit32 x{std::numeric_limits<std::uint32_t>::max()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{x.fee()});
}

{
FeeUnit32 x{std::numeric_limits<std::uint32_t>::min()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{x.fee()});
}

{
FeeUnit64 x{std::numeric_limits<std::uint64_t>::max()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y ==
Json::Value{std::numeric_limits<std::uint32_t>::max()});
}

{
FeeUnit64 x{std::numeric_limits<std::uint64_t>::min()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::uintValue);
BEAST_EXPECT(y == Json::Value{x.fee()});
}

{
FeeLevelDouble x{std::numeric_limits<double>::max()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::realValue);
BEAST_EXPECT(y == Json::Value{std::numeric_limits<double>::max()});
}

{
FeeLevelDouble x{std::numeric_limits<double>::min()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::realValue);
BEAST_EXPECT(y == Json::Value{std::numeric_limits<double>::min()});
}

{
XRPAmount x{std::numeric_limits<std::int64_t>::max()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::intValue);
BEAST_EXPECT(y ==
Json::Value{std::numeric_limits<std::int32_t>::max()});
}

{
XRPAmount x{std::numeric_limits<std::int64_t>::min()};
auto y = x.json();
BEAST_EXPECT(y.type() == Json::intValue);
BEAST_EXPECT(y ==
Json::Value{std::numeric_limits<std::int32_t>::min()});
}
}
};

Expand Down

1 comment on commit 5f61eff

@ripplelabs-jenkins
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jenkins Build Summary

Built from this commit

Built at 20190814 - 22:34:30

Test Results

Build Type Log Result Status
gcc.Debug
-Dcoverage=ON,
TARGET=coverage_report,
SKIP_TESTS=true
logfile no test results, t: 9s [BAD EXIT] FAIL πŸ”΄
gcc.Release
-Dassert=ON,
MANUAL_TESTS=true
logfile no test results, t: 13s [BAD EXIT] FAIL πŸ”΄
docs,
TARGET=docs
logfile 1 cases, 0 failed, t: 0m1s PASS βœ…
clang.Debug logfile no test results, t: 10s [BAD EXIT] FAIL πŸ”΄
gcc.Debug logfile no test results, t: 9s [BAD EXIT] FAIL πŸ”΄
clang.Debug
-Dunity=OFF
logfile no test results, t: 22s [BAD EXIT] FAIL πŸ”΄
clang.Release
-Dassert=ON
logfile no test results, t: 16s [BAD EXIT] FAIL πŸ”΄
gcc.Debug
-Dunity=OFF
logfile no test results, t: 20s [BAD EXIT] FAIL πŸ”΄
gcc.Debug
-Dstatic=OFF
logfile no test results, t: 9s [BAD EXIT] FAIL πŸ”΄
gcc.Release
-Dassert=ON
logfile no test results, t: 13s [BAD EXIT] FAIL πŸ”΄
gcc.Debug
-Dstatic=OFF -DBUILD_SHARED_LIBS=ON
logfile no test results, t: 9s [BAD EXIT] FAIL πŸ”΄
gcc.Debug,
NINJA_BUILD=true
logfile no test results, t: 25s [BAD EXIT] FAIL πŸ”΄
clang.Debug
-Dunity=OFF -Dsan=address,
PARALLEL_TESTS=false,
DEBUGGER=false
logfile no test results, t: 16s [BAD EXIT] FAIL πŸ”΄
clang.Debug
-Dunity=OFF -Dsan=undefined,
PARALLEL_TESTS=false
logfile no test results, t: 16s [BAD EXIT] FAIL πŸ”΄
msvc.Debug console no test results, t: n/a [BAD EXIT] FAIL πŸ”΄
msvc.Debug,
NINJA_BUILD=true
console no test results, t: n/a [BAD EXIT] FAIL πŸ”΄
msvc.Debug
-Dunity=OFF
console no test results, t: n/a [BAD EXIT] FAIL πŸ”΄
msvc.Release console no test results, t: n/a [BAD EXIT] FAIL πŸ”΄

Please sign in to comment.