Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ledger save/load test (RIPD-1378) #1955

Closed
wants to merge 4 commits into from
Closed

Add ledger save/load test (RIPD-1378) #1955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
50dee12
Add ledger save/load test (RIPD-1378)
mellery451 Jan 4, 2017
ffc64c5
[FOLD] Fix header format, append to vcproj file
mellery451 Jan 5, 2017
dd0f867
[FOLD] Add more startup ledger tests
mellery451 Jan 5, 2017
c667c81
[FOLD] Explicit size checks, improve fs error check
mellery451 Jan 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4156,6 +4156,10 @@
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\app\LedgerLoad_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\test\app\LoadFeeTrack_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
Expand Down
3 changes: 3 additions & 0 deletions Builds/VisualStudio2015/RippleD.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4914,6 +4914,9 @@
<ClCompile Include="..\..\src\test\app\HashRouter_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\app\LedgerLoad_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
<ClCompile Include="..\..\src\test\app\LoadFeeTrack_test.cpp">
<Filter>test\app</Filter>
</ClCompile>
Expand Down
229 changes: 229 additions & 0 deletions src/test/app/LedgerLoad_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2012-2017 Ripple Labs Inc.

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================

#include <BeastConfig.h>
#include <test/support/jtx.h>
#include <test/support/jtx/Env.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This #include may be redundant.

#include <ripple/beast/unit_test.h>
#include <ripple/protocol/JsonFields.h>
#include <ripple/protocol/SField.h>
#include <boost/scope_exit.hpp>
#include <boost/filesystem.hpp>
#include <boost/algorithm/string.hpp>
#include <fstream>

namespace ripple {

class LedgerLoad_test : public beast::unit_test::suite
{
Json::Value jvLedger_;
Json::Value jvHashes_;
boost::filesystem::path ledgerFile_;
boost::filesystem::path dbPath_;

auto ledgerConfig(std::string const& ledger, Config::StartUpType type)
{
assert(! dbPath_.empty());
auto p = std::make_unique<Config>();
test::setupConfigForUnitTests(*p);
p->START_LEDGER = ledger;
p->START_UP = type;
p->legacy("database_path", dbPath_.string());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Part of me wants you to make ledgerConfig a static function, and pass dbPath_ as another param, just so there can be no accidental calls before dbPath_ is initialized. One reason for that is that we don't have many other unit tests that have any state at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point - what about just adding an assert() to verify dbPath_ is set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's certainly simpler. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

idk about all that, the rules for test code are more relaxed compared to production code. We should not worry about "accidental" calls. For unit tests, we prioritize convenience and compactness of notation over robustness

return p;
}

// setup for test cases
void
setupLedger()
{
using namespace test::jtx;

boost::system::error_code ec;
// create a temporary path to write ledger files in
dbPath_ = boost::filesystem::temp_directory_path(ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;
dbPath_ /= boost::filesystem::unique_path("%%%%-%%%%-%%%%-%%%%", ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;
boost::filesystem::create_directories(dbPath_, ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;

ledgerFile_ = dbPath_ / "ledgerdata.json";

Env env {*this};
Account prev;

for(auto i = 0; i < 20; ++i)
{
Account acct {"A" + std::to_string(i)};
env.fund(XRP(10000), acct);
env.close();
if(i > 0)
{
env.trust(acct["USD"](1000), prev);
env(pay(acct, prev, acct["USD"](5)));
}
env(offer(acct, XRP(100), acct["USD"](1)));
env.close();
prev = std::move(acct);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC, building a new Account is expensive.

        Account prev;
        for(auto i = 0; i < 20; ++i)
        {
            Account acct {"A" + std::to_string(i)};
            env.fund(XRP(10000), acct);
            env.close();
            if(i > 0)
            {
                env.trust(acct["USD"](1000), prev);
                env(pay(acct, prev, acct["USD"](5)));
            }
            env(offer(acct, XRP(100), acct["USD"](1)));
            env.close();
            prev = std::move(acct);
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure - will use the move


jvLedger_ = env.rpc ("ledger", "current", "full") [jss::result];
BEAST_EXPECT(jvLedger_[jss::ledger][jss::accountState].size() == 101);

for(auto const& it : jvLedger_[jss::ledger][jss::accountState])
{
if(it[sfLedgerEntryType.fieldName] == "LedgerHashes")
{
jvHashes_ = it[sfHashes.fieldName];
}
}
BEAST_EXPECT(jvHashes_.size() == 41);

//write this ledger data to a file.
std::ofstream o (ledgerFile_.string(), std::ios::out | std::ios::trunc);
o << to_string(jvLedger_);
o.close();
}

void
testLoad ()
{
testcase ("Load a saved ledger");
using namespace test::jtx;

// create a new env with the ledger file specified for startup
Env env(*this, ledgerConfig(ledgerFile_.string(), Config::LOAD_FILE));
auto jrb = env.rpc ( "ledger", "current", "full") [jss::result];
BEAST_EXPECT(
jvLedger_[jss::ledger][jss::accountState].size() ==
jrb[jss::ledger][jss::accountState].size());
}

void
testBadFiles ()
{
testcase ("Load ledger: Bad Files");
using namespace test::jtx;
using namespace boost::filesystem;

// empty path
except ([this]
{
Env env(*this, ledgerConfig("", Config::LOAD_FILE));
});

// file does not exist
except ([this]
{
Env env(*this, ledgerConfig("badfile.json", Config::LOAD_FILE));
});

// make a corrupted version of the ledger file (last 10 bytes removed).
boost::system::error_code ec;
auto ledgerFileCorrupt = dbPath_ / "ledgerdata_bad.json";
copy_file(
ledgerFile_,
ledgerFileCorrupt,
copy_option::overwrite_if_exists,
ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;
auto filesize = file_size(ledgerFileCorrupt, ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;
resize_file(ledgerFileCorrupt, filesize - 10, ec);
if(! BEAST_EXPECTS(!ec, ec.message()))
return;

except ([this, &ledgerFileCorrupt]
{
Env env(*this, ledgerConfig(ledgerFileCorrupt.string(), Config::LOAD_FILE));
});
}

void
testLoadByHash ()
{
testcase ("Load by hash");
using namespace test::jtx;

// create a new env with the ledger hash specified for startup
auto ledgerHash = to_string(jvHashes_[jvHashes_.size()-1]);
boost::erase_all(ledgerHash, "\"");
Env env(*this, ledgerConfig(ledgerHash, Config::LOAD));
auto jrb = env.rpc ( "ledger", "current", "full") [jss::result];
BEAST_EXPECT(jrb[jss::ledger][jss::accountState].size() == 97);
BEAST_EXPECT(
jrb[jss::ledger][jss::accountState].size() <=
jvLedger_[jss::ledger][jss::accountState].size());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to not compare jrb...size() == 97 here?

}

void
testLoadLatest ()
{
testcase ("Load by keyword");
using namespace test::jtx;

// create a new env with the ledger "latest" specified for startup
Env env(*this, ledgerConfig("latest", Config::LOAD));
auto jrb = env.rpc ( "ledger", "current", "full") [jss::result];
BEAST_EXPECT(
jvLedger_[jss::ledger][jss::accountState].size() ==
jrb[jss::ledger][jss::accountState].size());
}

void
testLoadIndex ()
{
testcase ("Load by index");
using namespace test::jtx;

// create a new env with specific ledger index at startup
Env env(*this, ledgerConfig("43", Config::LOAD));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment does not match code. Maybe can get rid of comment. Or Start up the env at ledger 43

auto jrb = env.rpc ( "ledger", "current", "full") [jss::result];
BEAST_EXPECT(
jvLedger_[jss::ledger][jss::accountState].size() ==
jrb[jss::ledger][jss::accountState].size());
}

public:
void run ()
{
setupLedger();
BOOST_SCOPE_EXIT( this_ ) {
boost::system::error_code ec;
boost::filesystem::remove_all(this_->dbPath_, ec);
this_->expect(!ec, ec.message(), __FILE__, __LINE__);
} BOOST_SCOPE_EXIT_END
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a possible replacement with the same functionality but no macros:

        auto on_exit = [](auto this_)
        {
            boost::system::error_code ec;
            boost::filesystem::remove_all(this_->dbPath_, ec);
            this_->expect(!ec, ec.message(), __FILE__, __LINE__);
        };
        std::unique_ptr<LedgerLoad_test, decltype(on_exit)> hold_scope{this, on_exit};

Tested, seems to work fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and we can change this_ to this in the capture, right ?

Copy link
Contributor

@HowardHinnant HowardHinnant Jan 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think so. The deleter has to take a single LedgerLoad_test* as a parameter. Since we send this into the unique_ptr, then this gets passed to the deleter on exit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it - thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a little frustrating that make_unique is not available to help with this. We could write a little make_scope factory function around this idiom to play the role of make_unique. That's probably not worth it just for this one instance. But if we find ourselves doing this more places in the future, then it would be worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

C++17 auto deduction will then get rid of the need for make_scope. :-)

std::unique_ptr hold_scope{this, on_exit};


// test cases
testLoad ();
testBadFiles ();
testLoadByHash ();
testLoadLatest ();
testLoadIndex ();
}
};

BEAST_DEFINE_TESTSUITE (LedgerLoad, app, ripple);

} // ripple
3 changes: 2 additions & 1 deletion src/test/support/jtx/impl/Env.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ Env::AppBundle::AppBundle(beast::unit_test::suite& suite,
std::move(logs), std::move(timeKeeper_));
app = owned.get();
app->logs().threshold(beast::severities::kError);
app->setup();
if(! app->setup())
Throw<std::runtime_error> ("Env::AppBundle: setup failed");
timeKeeper->set(
app->getLedgerMaster().getClosedLedger()->info().closeTime);
app->doStart();
Expand Down
1 change: 1 addition & 0 deletions src/test/unity/app_test_unity.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <test/app/Flow_test.cpp>
#include <test/app/Freeze_test.cpp>
#include <test/app/HashRouter_test.cpp>
#include <test/app/LedgerLoad_test.cpp>
#include <test/app/LoadFeeTrack_test.cpp>
#include <test/app/MultiSign_test.cpp>
#include <test/app/OfferStream_test.cpp>
Expand Down