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

Integration test for node selection (PR) #397

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Integration test for node selection (PR) #397

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1abb56d
167: Add missing methods in Client. Add new test in ClientIntegration…
deyanzz Sep 28, 2023
894a45f
397: Remove comments
deyanzz Oct 5, 2023
191d650
167: Changes for test SetNetworkIsWorkingCorrectly.
deyanzz Oct 11, 2023
b8259a7
167: Resolve comments during PR.
deyanzz Oct 11, 2023
c1f5b98
167: Fix for failing pipeline.
deyanzz Oct 11, 2023
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
8 changes: 8 additions & 0 deletions sdk/main/include/Client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ class Client
*/
void close();

/**
* Replace all nodes in this Client with a new set of nodes from the Hedera network.
*
* @param networkMap The map with string representation of node addresses with their corresponding accountId.
* @return A reference to this Client object with the newly-set operator account ID from the map.
*/
Client& setNetwork(const std::unordered_map<std::string, AccountId>& networkMap);

/**
* Set the length of time a request sent by this Client can be processed before it times out.
*
Expand Down
9 changes: 9 additions & 0 deletions sdk/main/src/Client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ Client Client::forNetwork(const std::unordered_map<std::string, AccountId>& netw
{
Client client;
client.mImpl->mNetwork = std::make_shared<internal::Network>(internal::Network::forNetwork(networkMap));
client.mImpl->mMirrorNetwork = nullptr;
return client;
}

Expand Down Expand Up @@ -209,6 +210,14 @@ void Client::close()
}
}

//-----
Client& Client::setNetwork(const std::unordered_map<std::string, AccountId>& networkMap)
{
mImpl->mNetwork = std::make_shared<internal::Network>(internal::Network::forNetwork(networkMap));
mImpl->mMirrorNetwork = nullptr;
return *this;
}

//-----
Client& Client::setRequestTimeout(const std::chrono::duration<double>& timeout)
{
Expand Down
88 changes: 39 additions & 49 deletions sdk/tests/integration/ClientIntegrationTest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,41 @@
* limitations under the License.
*
*/
#include "AccountBalance.h"
#include "AccountBalanceQuery.h"
#include "AccountCreateTransaction.h"
#include "AccountId.h"
#include "AccountInfo.h"
#include "AccountInfoQuery.h"
#include "BaseIntegrationTest.h"
#include "Client.h"
#include "ED25519PrivateKey.h"
#include "Hbar.h"
#include "PublicKey.h"
#include "TransactionReceipt.h"
#include "TransactionRecord.h"
#include "TransactionResponse.h"
#include "TransferTransaction.h"
#include "exceptions/UninitializedException.h"
#include "impl/Utilities.h"

#include <chrono>
#include <filesystem>
#include <fstream>
#include <gtest/gtest.h>
#include <nlohmann/json.hpp>
#include <string>
#include <string_view>
#include <unordered_map>

using json = nlohmann::json;
using namespace std;
using namespace Hedera;

class ClientIntegrationTest : public ::testing::Test
class ClientIntegrationTest : public BaseIntegrationTest
{
protected:
// [[nodiscard]] inline const Client& getTestClient() const { return mClient; }
deyanzz marked this conversation as resolved.
Show resolved Hide resolved
[[nodiscard]] inline const std::string_view& getJsonNetworkTag() const { return mJsonNetworkTag; }
[[nodiscard]] inline const std::string_view& getJsonOperatorTag() const { return mJsonOperatorTag; }
[[nodiscard]] inline const std::string_view& getJsonAccountIdTag() const { return mJsonAccountIdTag; }
Expand All @@ -47,22 +61,14 @@ class ClientIntegrationTest : public ::testing::Test
[[nodiscard]] inline const AccountId& getAccountId() const { return mAccountId; }
[[nodiscard]] inline const std::string getPathToJSON() const { return mFilePath.string(); }

[[nodiscard]] inline const std::chrono::milliseconds getNegativeBackoffTime() const { return mNegativeBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getZeroBackoffTime() const { return mZeroBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getBelowMinBackoffTime() const { return mBelowMinBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getAboveMaxBackoffTime() const { return mAboveMaxBackoffTime; }

private:
// Client mClient;
deyanzz marked this conversation as resolved.
Show resolved Hide resolved

const std::string_view mJsonNetworkTag = "network";
const std::string_view mJsonOperatorTag = "operator";
const std::string_view mJsonAccountIdTag = "accountId";
const std::string_view mJsonPrivateKeyTag = "privateKey";

const std::chrono::milliseconds mNegativeBackoffTime = std::chrono::milliseconds(-1);
const std::chrono::milliseconds mZeroBackoffTime = std::chrono::milliseconds(0);
const std::chrono::milliseconds mBelowMinBackoffTime = DEFAULT_MIN_BACKOFF - std::chrono::milliseconds(1);
const std::chrono::milliseconds mAboveMaxBackoffTime = DEFAULT_MAX_BACKOFF + std::chrono::milliseconds(1);

const std::string_view mAccountIdStr = "0.0.3";
const AccountId mAccountId = AccountId::fromString("0.0.3");
const std::filesystem::path mFilePath = (std::filesystem::current_path() / "local_node.json").string();
Expand Down Expand Up @@ -124,52 +130,36 @@ TEST_F(ClientIntegrationTest, ConnectToLocalNode)
}

//-----
TEST_F(ClientIntegrationTest, SetInvalidMinBackoff)
TEST_F(ClientIntegrationTest, SetNetworkIsWorkingCorrectly)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);
const AccountId accountId_3 = AccountId::fromString("0.0.3");
const AccountId accountId_4 = AccountId::fromString("0.0.4");
const AccountId accountId_5 = AccountId::fromString("0.0.5");
const AccountId accountId_6 = AccountId::fromString("0.0.6");
const AccountId accountId_7 = AccountId::fromString("0.0.7");

AccountBalance accountBalance_3;
AccountBalance accountBalance_4;
AccountBalance accountBalance_5;
AccountBalance accountBalance_6;
AccountBalance accountBalance_7;

// When / Then
EXPECT_THROW(client.setMinBackoff(getNegativeBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMinBackoff(getAboveMaxBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
}

//-----
TEST_F(ClientIntegrationTest, SetValidMinBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_NO_THROW(client.setMinBackoff(getZeroBackoffTime()));
EXPECT_NO_THROW(client.setMinBackoff(DEFAULT_MIN_BACKOFF));
EXPECT_NO_THROW(client.setMinBackoff(DEFAULT_MAX_BACKOFF));
}
networkMap.insert(std::pair<std::string, AccountId>("34.94.106.61:50211", accountId_3));
networkMap.insert(std::pair<std::string, AccountId>("35.237.119.55:50211", accountId_4));
networkMap.insert(std::pair<std::string, AccountId>("35.245.27.193:50211", accountId_5));

//-----
TEST_F(ClientIntegrationTest, SetInvalidMaxBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_THROW(client.setMaxBackoff(getNegativeBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getZeroBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getBelowMinBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getAboveMaxBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
}
std::unordered_map<std::string, AccountId> newNetworkMap;
newNetworkMap.insert(std::pair<std::string, AccountId>("35.237.119.55:50211", accountId_6));
newNetworkMap.insert(std::pair<std::string, AccountId>("35.245.27.193:50211", accountId_7));

//-----
TEST_F(ClientIntegrationTest, SetValidMaxBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);
client.setNetwork(newNetworkMap);

// When / Then
EXPECT_NO_THROW(client.setMaxBackoff(DEFAULT_MIN_BACKOFF));
EXPECT_NO_THROW(client.setMaxBackoff(DEFAULT_MAX_BACKOFF));
}
ASSERT_NO_THROW(accountBalance_6 = AccountBalanceQuery().setAccountId(accountId_6).execute(client));
ASSERT_NO_THROW(accountBalance_7 = AccountBalanceQuery().setAccountId(accountId_7).execute(client));
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine for now but I think we should really try to enforce going forward that the integration tests do not communicate with previewnet/testnet/mainnet nodes, hence my previous comments about waiting for the multi-node setup to be complete. A user should be able to test locally with that set-up if they so desire, and seeing as how this test contains URLs for testnet consensus nodes, these tests would not pass.

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 totally agree with you - using hardcoded URLs in the integration tests is a bad practice.
Integration tests should communicate only with the local node and this approach should be used in all of the Hedera's SDKs.
For now I will suggest to add a new JSON config files containing all of the consensus nodes for mainnet / previewnet / test. I will track this change in #525.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary as we should not be communicating with testnet nodes to begin with. We would add this JSON file ultimately to remove it when we get the multi-node setup integrated. I think we should leave it for now and then go through this again when we get the multi-node setup working.

}
61 changes: 61 additions & 0 deletions sdk/tests/unit/ClientTest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,20 @@ class ClientTest : public ::testing::Test
return mTestNetworkUpdatePeriod;
}

[[nodiscard]] inline const std::chrono::milliseconds getNegativeBackoffTime() const { return mNegativeBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getZeroBackoffTime() const { return mZeroBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getBelowMinBackoffTime() const { return mBelowMinBackoffTime; }
[[nodiscard]] inline const std::chrono::milliseconds getAboveMaxBackoffTime() const { return mAboveMaxBackoffTime; }

private:
const AccountId mAccountId = AccountId(10ULL);
const std::unique_ptr<ED25519PrivateKey> mPrivateKey = ED25519PrivateKey::generatePrivateKey();
const std::chrono::duration<double> mTestNetworkUpdatePeriod = std::chrono::seconds(2);

const std::chrono::milliseconds mNegativeBackoffTime = std::chrono::milliseconds(-1);
const std::chrono::milliseconds mZeroBackoffTime = std::chrono::milliseconds(0);
const std::chrono::milliseconds mBelowMinBackoffTime = DEFAULT_MIN_BACKOFF - std::chrono::milliseconds(1);
const std::chrono::milliseconds mAboveMaxBackoffTime = DEFAULT_MAX_BACKOFF + std::chrono::milliseconds(1);
};

//-----
Expand Down Expand Up @@ -108,3 +118,54 @@ TEST_F(ClientTest, SetNetworkUpdatePeriod)
// Then
EXPECT_EQ(client.getNetworkUpdatePeriod(), getTestNetworkUpdatePeriod());
}

//-----
TEST_F(ClientTest, SetInvalidMinBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_THROW(client.setMinBackoff(getNegativeBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMinBackoff(getAboveMaxBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
}

//-----
TEST_F(ClientTest, SetValidMinBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_NO_THROW(client.setMinBackoff(getZeroBackoffTime()));
EXPECT_NO_THROW(client.setMinBackoff(DEFAULT_MIN_BACKOFF));
EXPECT_NO_THROW(client.setMinBackoff(DEFAULT_MAX_BACKOFF));
}

//-----
TEST_F(ClientTest, SetInvalidMaxBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_THROW(client.setMaxBackoff(getNegativeBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getZeroBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getBelowMinBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
EXPECT_THROW(client.setMaxBackoff(getAboveMaxBackoffTime()), std::invalid_argument); // INVALID_ARGUMENT
}

//-----
TEST_F(ClientTest, SetValidMaxBackoff)
{
// Given
std::unordered_map<std::string, AccountId> networkMap;
Client client = Client::forNetwork(networkMap);

// When / Then
EXPECT_NO_THROW(client.setMaxBackoff(DEFAULT_MIN_BACKOFF));
EXPECT_NO_THROW(client.setMaxBackoff(DEFAULT_MAX_BACKOFF));
}
Loading