From f95961e212fab231d28dfd5676797aae4da479bb Mon Sep 17 00:00:00 2001 From: marta-lokhova Date: Wed, 7 Aug 2024 10:35:57 -0700 Subject: [PATCH] tests: harder simulation tests --- lib/libmedida | 2 +- src/overlay/test/OverlayTests.cpp | 13 +++++++++---- src/overlay/test/SurveyManagerTests.cpp | 25 ++++++++++++++----------- src/simulation/Simulation.cpp | 18 ------------------ src/simulation/Simulation.h | 7 +++++++ 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/lib/libmedida b/lib/libmedida index f91354b005..299c6f6c89 160000 --- a/lib/libmedida +++ b/lib/libmedida @@ -1 +1 @@ -Subproject commit f91354b0055de939779d392999975d611b1b1ad5 +Subproject commit 299c6f6c897961fa123c455151a437325c291484 diff --git a/src/overlay/test/OverlayTests.cpp b/src/overlay/test/OverlayTests.cpp index 2c813f5c82..45da9a9460 100644 --- a/src/overlay/test/OverlayTests.cpp +++ b/src/overlay/test/OverlayTests.cpp @@ -2325,12 +2325,13 @@ TEST_CASE("peer is purged from database after few failures", REQUIRE(!peerManager.load(localhost(cfg2.PEER_PORT)).second); } -TEST_CASE("disconnected topology recovery") +TEST_CASE("disconnected topology recovery", "[overlay][simulation]") { + auto initCfg = getTestConfig(); auto cfgs = std::vector{}; auto peers = std::vector{}; - for (int i = 0; i < 8; ++i) + for (int i = 0; i < 7; ++i) { auto cfg = getTestConfig(i + 1); cfgs.push_back(cfg); @@ -2340,8 +2341,12 @@ TEST_CASE("disconnected topology recovery") auto doTest = [&](bool usePreferred) { auto simulation = Topologies::separate( 7, 0.5, Simulation::OVER_LOOPBACK, - sha256(getTestConfig().NETWORK_PASSPHRASE), 0, [&](int i) { - auto cfg = cfgs[i]; + sha256(initCfg.NETWORK_PASSPHRASE), 0, [&](int i) { + if (i == 0) + { + return initCfg; + } + auto cfg = cfgs[i - 1]; cfg.TARGET_PEER_CONNECTIONS = 1; if (usePreferred) { diff --git a/src/overlay/test/SurveyManagerTests.cpp b/src/overlay/test/SurveyManagerTests.cpp index a27ccf18ab..2cf6a45bca 100644 --- a/src/overlay/test/SurveyManagerTests.cpp +++ b/src/overlay/test/SurveyManagerTests.cpp @@ -109,10 +109,10 @@ setupStaticNetworkTopology(std::vector& configList, } // D->A->B->C B->E - simulation->addConnection(keyList[D], keyList[A]); - simulation->addConnection(keyList[A], keyList[B]); - simulation->addConnection(keyList[B], keyList[C]); - simulation->addConnection(keyList[B], keyList[E]); + simulation->addPendingConnection(keyList[D], keyList[A]); + simulation->addPendingConnection(keyList[A], keyList[B]); + simulation->addPendingConnection(keyList[B], keyList[C]); + simulation->addPendingConnection(keyList[B], keyList[E]); simulation->startAllNodes(); @@ -402,7 +402,7 @@ TEST_CASE("survey request process order", "[overlay][survey][topology]") { for (int j = i + 1; j < numberOfNodes; j++) { - simulation->addConnection(keyList[i], keyList[j]); + simulation->addPendingConnection(keyList[i], keyList[j]); } } @@ -731,10 +731,10 @@ TEST_CASE("Time sliced dynamic topology survey", "[overlay][survey][topology]") } // D->A->B->C B->E (F not connected) - simulation->addConnection(keyList[D], keyList[A]); - simulation->addConnection(keyList[A], keyList[B]); - simulation->addConnection(keyList[B], keyList[C]); - simulation->addConnection(keyList[B], keyList[E]); + simulation->addPendingConnection(keyList[D], keyList[A]); + simulation->addPendingConnection(keyList[A], keyList[B]); + simulation->addPendingConnection(keyList[B], keyList[C]); + simulation->addPendingConnection(keyList[B], keyList[E]); simulation->startAllNodes(); @@ -785,10 +785,13 @@ TEST_CASE("Time sliced dynamic topology survey", "[overlay][survey][topology]") checkSurveyState(nonce, /*isReporting*/ false, {A, B, C, D, E}); // Add F to the simulation and connect to B - simulation->addNode(configList.at(F).NODE_SEED, qSet, &configList.at(F)); - simulation->getNode(keyList[F])->start(); + simulation->addNode(configList.at(F).NODE_SEED, qSet, &configList.at(F)) + ->start(); simulation->addConnection(keyList[F], keyList[B]); + // Let survey run for a bit to establish connection + crankForSurvey(); + // Disconnect E from B simulation->dropConnection(keyList[B], keyList[E]); diff --git a/src/simulation/Simulation.cpp b/src/simulation/Simulation.cpp index bda019fa8f..0818922f6a 100644 --- a/src/simulation/Simulation.cpp +++ b/src/simulation/Simulation.cpp @@ -45,12 +45,6 @@ Simulation::Simulation(Mode mode, Hash const& networkID, ConfigGen confGen, parallel = parallel && mVirtualClockMode == VirtualClock::REAL_TIME; mIdleApp = Application::create(mClock, cfg); mPeerMap.emplace(mIdleApp->getConfig().PEER_PORT, mIdleApp); - - // mIdleApp can end up connected to some nodes in the simulation, but never - // starts Herder. Therefore, it needs to initialize the max tx size so that - // an assertion doesn't fail when it initializes flow control. - Herder& herder = mIdleApp->getHerder(); - herder.setMaxTxSize(herder.getMaxClassicTxSize()); } Simulation::~Simulation() @@ -152,18 +146,6 @@ Simulation::addNode(SecretKey nodeKey, SCPQuorumSet qSet, Config const* cfg2, mPeerMap.emplace(app->getConfig().PEER_PORT, std::weak_ptr(app)); - - Herder& herder = app->getHerder(); - if (herder.getState() == Herder::HERDER_BOOTING_STATE) - { - // Application creation did not start Herder. To protect against cases - // where the test using this new node run the node's OverlayManager - // without starting Herder, we initialize the max tx size here. This - // prevents an assertion failure on flow control initialization. This is - // harmless if the test later starts Herder as `HerderImpl::start` will - // overwrite this value. - herder.setMaxTxSize(herder.getMaxClassicTxSize()); - } return app; } diff --git a/src/simulation/Simulation.h b/src/simulation/Simulation.h index cab300a425..8743af37f2 100644 --- a/src/simulation/Simulation.h +++ b/src/simulation/Simulation.h @@ -47,6 +47,8 @@ class Simulation void setCurrentVirtualTime(VirtualClock::time_point t); void setCurrentVirtualTime(VirtualClock::system_time_point t); + // Add new node to the simulation. This function does not start the node. + // Callers are expected to call `start` or `startAllNodes` manually. Application::pointer addNode(SecretKey nodeKey, SCPQuorumSet qSet, Config const* cfg = nullptr, bool newDB = true, uint32_t startAtLedger = 0, @@ -55,6 +57,9 @@ class Simulation std::vector getNodes(); std::vector getNodeIDs(); + // Add a pending connection to an unstarted node. Typically called after + // `addNode`, but before `startAllNodes`. No-op if the simulation is already + // started. void addPendingConnection(NodeID const& initiator, NodeID const& acceptor); // Returns LoopbackPeerConnection given initiator, acceptor pair or nullptr std::shared_ptr @@ -80,6 +85,8 @@ class Simulation void crankUntil(VirtualClock::system_time_point timePoint, bool finalCrank); std::string metricsSummary(std::string domain = ""); + // Add a real (not pending) connection to the simulation. Works even if the + // simulation has started. void addConnection(NodeID initiator, NodeID acceptor); void dropConnection(NodeID initiator, NodeID acceptor); Config newConfig(); // generates a new config