add cli wallet test framework #675

jmjatlanta · 2018-02-19T12:27:26Z

These are tests and helper functions for testing wallet functionality. Please review and feel free to comment about improvements.

This was originally to test issue 576, but a new ticket was created (674) for cleanup of the test code.

oxarbitrage · 2018-02-19T15:15:01Z

relating #576

oxarbitrage · 2018-02-19T15:17:21Z

very useful!

oxarbitrage

please pass --seed-nodes "[]" to the start of the witness node to avoid connecting to the live blockchain seeds. this is a private testnet setup and need to pass empty seeds. thanks!

oxarbitrage · 2018-02-19T21:33:47Z

with the empty seed-nodes the tests will be quicker as the node does not need to connect to those remote addresses.

oxarbitrage · 2018-02-19T21:34:23Z

tests/cli/main.cpp

+      init1_obj = wapiptr->get_witness("init1");
+
+      int init2_middle_votes = init2_obj.total_votes;
+      BOOST_CHECK(init2_middle_votes > init2_start_votes);


this test is failing.

I added a check to make sure the maintenance interval is set to a low value for this test. Is there a way to adjust the maintenance interval at run time?

this can be done by another way, check for example:

https://github.com/bitshares/bitshares-core/blob/master/tests/tests/swan_tests.cpp#L114

in there, block production advances until a hardfork time, you can play with that in order to advance the chain to the time you need. it will be something like:

vote for witness
advance until next maintenance
check for vote

looking good :)

Take a look at https://github.com/jmjatlanta/bitshares-core/blob/b6a6dc3a1f3b3af1f8b3e65f2392863fb306670c/tests/cli/main.cpp#L113

It is a modified version of what is in database_fixture. No more playing with the default maintenance interval

oxarbitrage · 2018-02-19T21:34:51Z

tests/cli/main.cpp

+
+      // set the voting proxy to nathan
+      BOOST_TEST_MESSAGE("About to set voting proxy.");
+      signed_transaction voting_tx = wapiptr->set_voting_proxy("jmjatlanta", "nathan", true);


make test to check if the proxy changed.

actually, i think they should be separated test than the cli_vote_for_2_witnesses , something like proxy_update will do it.
in cli_vote_for_2_witnesses only add the needed stuff for that test. we will add more as we need them in different test cases.

Created a new test for testing voting proxy

jmjatlanta · 2018-02-22T12:29:48Z

tests/cli/main.cpp

+/**
+ * Start a server and connect using the same calls as the CLI
+ */
+BOOST_AUTO_TEST_CASE( cli_set_voting_proxy )


Separated the proxy into its own test

jmjatlanta · 2018-02-22T12:39:36Z

tests/cli/main.cpp

+    boost::program_options::variables_map cfg;
+    cfg.emplace("rpc-endpoint", boost::program_options::variable_value(string("127.0.0.1:8090"), false));
+    cfg.emplace("genesis-json", boost::program_options::variable_value(create_genesis_file(app_dir), false));
+    cfg.emplace("seed-nodes", boost::program_options::variable_value(string("[]"), false));


No longer attempting to connect to seed nodes during test

good, thanks.

oxarbitrage

looks good to me now John. thank you for the changes.

oxarbitrage · 2018-02-26T15:07:09Z

tests/cli/main.cpp

+      // set the voting proxy to nathan
+      BOOST_TEST_MESSAGE("About to set voting proxy.");
+      signed_transaction voting_tx = wapiptr->set_voting_proxy("jmjatlanta", "nathan", true);
+


please confirm the proxy is now nathan for jmatlanta with BOOST_CHECK. every change should be followed by a test if possible.

abitmore

It's good that the tests are working. However, it's best if we can abstract the common procedures for ease of future use.

abitmore · 2018-03-12T13:40:09Z

tests/cli/main.cpp

@@ -0,0 +1,388 @@
+/*
+ * Copyright (c) 2015 Cryptonomex, Inc., and contributors.


IMHO better replace Cryptonomex with your own name and replace year to 2018.

abitmore · 2018-03-12T13:45:00Z

tests/cli/main.cpp

+    app1->register_plugin< graphene::witness_plugin::witness_plugin >();
+    app1->startup_plugins();
+    boost::program_options::variables_map cfg;
+    cfg.emplace("rpc-endpoint", boost::program_options::variable_value(string("127.0.0.1:8090"), false));


please use an available random port.

abitmore · 2018-03-12T13:53:16Z

tests/cli/main.cpp

+	boost::filesystem::path genesis_path = boost::filesystem::path{directory.path().generic_string()} / "genesis.json";
+    fc::path genesis_out = genesis_path;
+    graphene::chain::genesis_state_type genesis_state = graphene::app::detail::create_example_genesis();
+    std::cerr << "Creating example genesis state in file " << genesis_out.generic_string() << "\n";


Why output to std::cerr?

abitmore · 2018-03-12T13:55:16Z

tests/cli/main.cpp

+      // connect to the server
+      graphene::wallet::wallet_data wdata;
+      wdata.chain_id = app1->chain_database()->get_chain_id();
+      wdata.ws_server = "ws://127.0.0.1:8090";


Replace the port with the chosen random port.

abitmore · 2018-03-12T13:56:35Z

tests/cli/main.cpp

+      auto apic = std::make_shared<fc::rpc::websocket_api_connection>(*con);
+
+      auto remote_api = apic->get_remote_api< login_api >(1);
+      BOOST_CHECK(remote_api->login( wdata.ws_user, wdata.ws_password ) );


Please reuse the code. Almost every test case need to connect first.

Created a class that connects and provides all connectivity objects

abitmore · 2018-03-12T13:58:44Z

tests/cli/main.cpp

+    fc::path genesis_out = genesis_path;
+    graphene::chain::genesis_state_type genesis_state = graphene::app::detail::create_example_genesis();
+    std::cerr << "Creating example genesis state in file " << genesis_out.generic_string() << "\n";
+    fc::json::save_to_file(genesis_state, genesis_out);


After generated the genesis file, please replace all Public keys with a new random key, then use it in later tests.

I am unfamiliar with the PKI architecture in Bitshares. I am working on familiarizing myself with this now.

abitmore

Mostly good so far. May need to reorganize (e.g. split it to several files) in the future.

By the way, please resolve conflicts.

abitmore · 2018-03-15T17:28:16Z

tests/cli/main.cpp

+}
+
+/***
+ * @brief Start the application, listening on port 8090


Please replace the 8090 here.

abitmore · 2018-03-15T17:30:09Z

tests/cli/main.cpp

+ */
+bool generate_block(std::shared_ptr<graphene::app::application> app) {
+	try {
+	    fc::ecc::private_key committee_key = fc::ecc::private_key::regenerate(fc::sha256::hash(string("nathan")));


Can use the same method to generate a new key and replace the keys generated in genesis file.

Still working on this...

abitmore · 2018-03-15T17:31:55Z

tests/cli/main.cpp

+   using namespace graphene::app;
+   std::shared_ptr<graphene::app::application> app1;
+   try {
+	   fc::temp_directory app_dir ( graphene::utilities::temp_directory_path() );


Please replace tabs with spaces.

3 spaces for indentation.

Again!?!??!! Cleaned up.

outdated

abitmore · 2018-03-20T21:31:04Z

tests/cli/main.cpp

@@ -51,18 +51,49 @@
 * Helper Methods
 *********************/

-// hack:  import create_example_genesis() even though it's a way, way
-// specific internal detail
+/////////


I'm not sure whether Doxygen recognize this format. It does recognize /** */.

Yes, it is recognized. https://www.stack.nl/~dimitri/doxygen/manual/docblocks.html

I prefer /* */, but some like // as it allows you to comment out big chunks of code.

According to the link, maybe need 3 slashes?

///////////// /// doc ///////////

Yes, you are correct. I have modified the areas that should be doxygen'd.

jmjatlanta mentioned this pull request Feb 19, 2018

Unit test framework for testing CLI #674

Closed

oxarbitrage changed the title ~~Issue 674 and 576~~ add cli wallet test framework Feb 19, 2018

oxarbitrage self-requested a review February 19, 2018 15:17

oxarbitrage suggested changes Feb 19, 2018

View reviewed changes

oxarbitrage mentioned this pull request Feb 19, 2018

remove wdumps from database_api.cpp and wallet.cpp #680

Merged

oxarbitrage reviewed Feb 19, 2018

View reviewed changes

jmjatlanta added 5 commits February 22, 2018 05:49

Beginnings of a test case

5b3ee10

Modified test case to replicate the problem

8f2d468

Modified tests to send votes

bc6c470

testing and displaying results for debugging

c544f5a

test code cleanup

4f6b0f8

jmjatlanta force-pushed the Issue_576 branch from 2cc12d9 to 4f6b0f8 Compare February 22, 2018 10:49

separate test for proxy voting

f545941

jmjatlanta commented Feb 22, 2018

View reviewed changes

jmjatlanta added 3 commits February 22, 2018 09:10

Added check for maintenance interval

218a073

Generating maint. blocks without modifying default maintenance interval

b6a6dc3

checking to be sure set_voting_proxy changed something

11fae0a

oxarbitrage previously approved these changes Feb 26, 2018

View reviewed changes

abitmore requested changes Mar 12, 2018

View reviewed changes

tests use random rpc port, wrapped client connect code in class

9bd3e5b

abitmore requested changes Mar 15, 2018

View reviewed changes

jmjatlanta added 2 commits March 15, 2018 14:15

Replace tabs with spaces

aaee5ad

Resolved conflict between PR and develop branch

adde3fa

abitmore reviewed Mar 20, 2018

View reviewed changes

adjusted comments for doxygen compatibility

f8edc28

abitmore approved these changes Mar 21, 2018

View reviewed changes

abitmore added this to the 201803 Non-Consensus-Changing Release milestone Mar 21, 2018

abitmore merged commit 39cf1c3 into bitshares:develop Mar 21, 2018

jmjatlanta deleted the Issue_576 branch March 21, 2018 14:13

		@@ -0,0 +1,388 @@
		/*
		* Copyright (c) 2015 Cryptonomex, Inc., and contributors.

add cli wallet test framework #675

add cli wallet test framework #675

Conversation

jmjatlanta commented Feb 19, 2018

oxarbitrage commented Feb 19, 2018

oxarbitrage commented Feb 19, 2018

oxarbitrage left a comment

Choose a reason for hiding this comment

oxarbitrage commented Feb 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxarbitrage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abitmore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmjatlanta Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

abitmore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmjatlanta Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

jmjatlanta Mar 15, 2018 •

edited

Loading

jmjatlanta Mar 21, 2018 •

edited

Loading