-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
[GRPH-3] port cli tests from bitshares to Peerplays #105
Conversation
This is the same as #98 |
@oxarbitrage GRPH-3 was assigned to me. I've raised this PR as part of it. Moreover, it covers more tests compare to #98 |
@srpatel19590 - Which are the additional tests that you mentioned ? |
@bobinson Here is the list of additional tests.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix the merge conflicts.
@@ -590,6 +595,12 @@ class wallet_api | |||
*/ | |||
bool load_wallet_file(string wallet_filename = ""); | |||
|
|||
/** Quitting from BitShares wallet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srpatel19590 - This should be Peerplays wallet
tests/CMakeLists.txt
Outdated
file(GLOB CLI_SOURCES "cli/*.cpp") | ||
add_executable( cli_test ${CLI_SOURCES} ) | ||
if(WIN32) | ||
list(APPEND PLATFORM_SPECIFIC_LIBS ws2_32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srpatel19590 "PLATFORM_SPECIFIC_LIBS ws2_32" -- what are these libraries ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are windows socket lib. Additionally added to PLATFORM_SPECIFIC_LIBS when the target is windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@srpatel19590 - provide specific details. ie link, versions etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bobinson By compiling with Visual Studio 2017 and 2015 Update 1, the required ws2_32 lib will already available and we just required to link it. So MSVS 2017 and 2015 Update 1 is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this part if we are not using WIN32. Moreover we can use below instruction to compile in windows.
https://github.com/bitshares/bitshares-core/wiki/BUILD_WIN32#windows-development-environment-setup-and-build-instructions
b5ec44f
to
b16f4e6
Compare
As we merged #98 which similar i think this pull should be closed and open a new one for the test cases we left out of we need them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create a fresh PR with the tests.
217db8c
to
811738f
Compare
@bobinson @oxarbitrage PR is updated with just additional cli tests and its fixes |
send a clean PR after addressing the dependencies. |
Incorporated cli tests from bitshares and related bug fixes and/or enhancements. It requires to add #99 to run
chain_test