diff options
author | MacroFake <falke.marco@gmail.com> | 2022-08-05 15:33:27 +0200 |
---|---|---|
committer | MacroFake <falke.marco@gmail.com> | 2022-08-05 15:33:45 +0200 |
commit | 006740b6f6475ed6ae08803c60dd82027602695b (patch) | |
tree | 34a072f8859fcd5b6e4dac402c456fd6e4bc2e77 /src/test | |
parent | 92f6461cfd39fff2fc885dd623fa47e7d8d53827 (diff) | |
parent | a23cca56c0a7f4a267915b4beba3af3454c51603 (diff) | |
download | bitcoin-006740b6f6475ed6ae08803c60dd82027602695b.tar.xz |
Merge bitcoin/bitcoin#25721: refactor: Replace BResult with util::Result
a23cca56c0a7f4a267915b4beba3af3454c51603 refactor: Replace BResult with util::Result (Ryan Ofsky)
Pull request description:
Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in https://github.com/bitcoin/bitcoin/pull/25665.
This change makes the following improvements originally implemented in https://github.com/bitcoin/bitcoin/pull/25665:
- More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value.
- Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors.
- More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent.
- Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?)
- Has unit tests.
ACKs for top commit:
MarcoFalke:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603 🏵
jonatack:
ACK a23cca56c0a7f4a267915b4beba3af3454c51603
Tree-SHA512: 2769791e08cd62f21d850aa13fa7afce4fb6875a9cedc39ad5025150dbc611c2ecfd7b3aba8b980a79fde7fbda13babdfa37340633c69b501b6e89727bad5b31
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/result_tests.cpp | 96 | ||||
-rw-r--r-- | src/test/util/wallet.cpp | 6 |
2 files changed, 98 insertions, 4 deletions
diff --git a/src/test/result_tests.cpp b/src/test/result_tests.cpp new file mode 100644 index 0000000000..6a23a7b895 --- /dev/null +++ b/src/test/result_tests.cpp @@ -0,0 +1,96 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <util/result.h> + +#include <boost/test/unit_test.hpp> + +inline bool operator==(const bilingual_str& a, const bilingual_str& b) +{ + return a.original == b.original && a.translated == b.translated; +} + +inline std::ostream& operator<<(std::ostream& os, const bilingual_str& s) +{ + return os << "bilingual_str('" << s.original << "' , '" << s.translated << "')"; +} + +BOOST_AUTO_TEST_SUITE(result_tests) + +struct NoCopy { + NoCopy(int n) : m_n{std::make_unique<int>(n)} {} + std::unique_ptr<int> m_n; +}; + +bool operator==(const NoCopy& a, const NoCopy& b) +{ + return *a.m_n == *b.m_n; +} + +std::ostream& operator<<(std::ostream& os, const NoCopy& o) +{ + return os << "NoCopy(" << *o.m_n << ")"; +} + +util::Result<int> IntFn(int i, bool success) +{ + if (success) return i; + return util::Error{Untranslated(strprintf("int %i error.", i))}; +} + +util::Result<bilingual_str> StrFn(bilingual_str s, bool success) +{ + if (success) return s; + return util::Error{strprintf(Untranslated("str %s error."), s.original)}; +} + +util::Result<NoCopy> NoCopyFn(int i, bool success) +{ + if (success) return {i}; + return util::Error{Untranslated(strprintf("nocopy %i error.", i))}; +} + +template <typename T> +void ExpectResult(const util::Result<T>& result, bool success, const bilingual_str& str) +{ + BOOST_CHECK_EQUAL(bool(result), success); + BOOST_CHECK_EQUAL(util::ErrorString(result), str); +} + +template <typename T, typename... Args> +void ExpectSuccess(const util::Result<T>& result, const bilingual_str& str, Args&&... args) +{ + ExpectResult(result, true, str); + BOOST_CHECK_EQUAL(result.has_value(), true); + BOOST_CHECK_EQUAL(result.value(), T{std::forward<Args>(args)...}); + BOOST_CHECK_EQUAL(&result.value(), &*result); +} + +template <typename T, typename... Args> +void ExpectFail(const util::Result<T>& result, const bilingual_str& str) +{ + ExpectResult(result, false, str); +} + +BOOST_AUTO_TEST_CASE(check_returned) +{ + ExpectSuccess(IntFn(5, true), {}, 5); + ExpectFail(IntFn(5, false), Untranslated("int 5 error.")); + ExpectSuccess(NoCopyFn(5, true), {}, 5); + ExpectFail(NoCopyFn(5, false), Untranslated("nocopy 5 error.")); + ExpectSuccess(StrFn(Untranslated("S"), true), {}, Untranslated("S")); + ExpectFail(StrFn(Untranslated("S"), false), Untranslated("str S error.")); +} + +BOOST_AUTO_TEST_CASE(check_value_or) +{ + BOOST_CHECK_EQUAL(IntFn(10, true).value_or(20), 10); + BOOST_CHECK_EQUAL(IntFn(10, false).value_or(20), 20); + BOOST_CHECK_EQUAL(NoCopyFn(10, true).value_or(20), 10); + BOOST_CHECK_EQUAL(NoCopyFn(10, false).value_or(20), 20); + BOOST_CHECK_EQUAL(StrFn(Untranslated("A"), true).value_or(Untranslated("B")), Untranslated("A")); + BOOST_CHECK_EQUAL(StrFn(Untranslated("A"), false).value_or(Untranslated("B")), Untranslated("B")); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/wallet.cpp b/src/test/util/wallet.cpp index 7a00ac9e1f..b54774cbb9 100644 --- a/src/test/util/wallet.cpp +++ b/src/test/util/wallet.cpp @@ -8,6 +8,7 @@ #include <outputtype.h> #include <script/standard.h> #ifdef ENABLE_WALLET +#include <util/check.h> #include <util/translation.h> #include <wallet/wallet.h> #endif @@ -20,10 +21,7 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq std::string getnewaddress(CWallet& w) { constexpr auto output_type = OutputType::BECH32; - auto op_dest = w.GetNewDestination(output_type, ""); - assert(op_dest.HasRes()); - - return EncodeDestination(op_dest.GetObj()); + return EncodeDestination(*Assert(w.GetNewDestination(output_type, ""))); } #endif // ENABLE_WALLET |