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.

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

Add API test harness for Linux & OSX #4366

Merged
merged 88 commits into from Aug 24, 2020
Merged

Conversation

ghubstan
Copy link
Member

@ghubstan ghubstan commented Jul 9, 2020

This PR adds a new :apitest subproject to Bisq with the following goals:

  • Enable test driven development of the API project
  • Automate end to end testing of the API
  • Simplify :desktop testing in regtest/full-dao mode

It requires the installation of bitcoin-core v0.19 or v0.20 binaries in the $PATH, or the path to them defined in the subproject's config file src/main/resources/apitest.properties:

bitcoinPath=/home/you/bitcoin/v0.19.1/src

The :apitest build downloads and installs dao-setup files for bitcoind, bob & alice datadirs in apitest/build/resources/main, and test case @BeforeAll methods start bitcoind, seednode, arbitration node (:desktop or :daemon), plus bob & alice nodes (:desktop or :daemon) as Linux background processes. Currently, the only testing mode supported is regtest / full-dao mode, without Tor.

To run a full clean build, and install dao setup files:

./gradlew clean build :apitest:installDaoSetup

To install or re-install dao setup files:

./gradlew :apitest:installDaoSetup

To run API @Test cases:

./gradlew :apitest:test -DrunApiTests=true

A gradle Test Summary containing any initialization errors and test failures can be found in

apitest/build/reports/tests/test/index.html

This test harness works on a 2014 Mac mini with a Dual-Core i5 and 8 Gb RAM.

[EDITED 27-July-2020]

ApiTestConfig works like :common Config, but specific to this subproject.

BisqAppConfig is an enumeration specifying Bisq desktop and daemon
options for running seednode, arbnode, bob & alices nodes in regtest /
full-dao mode.
* apitest.properties - config file for customizing ApiTestConfig
  options

* logback.xml - logging config file, will override logback files
  found in classpath

* bitcoin.conf - bitcoin-core regtest config file, overwritten
  during startup with correct path to blocknofity

* blocknotify - bitcoin-core blocknotify config file
Finding the pid of background linux/java processes has been
difficult to do from java, so a bash script is provided to
simplify the task.
The apitest.linux package is for running random bash commands,
running 'bitcoind -regtest', running random 'bitcoin-cli -regtest'
commands, and spinning up Bisq apps such as seednode, arbnode,
and bob & alice nodes.

All but random bash and bitcoin-cli commands are run in the background.

The bitcoin-cli response processing is crude;  a more sophiticated
bitcoin-core rpc interface is not in the scope of this PR.
The driver class uses an ExecutorService to submit Callable
tasks for starting bitcoind and Bisq nodes as Linux background
processes.

By default, ApiTestMain starts background processes to support
regtest/dao testing, runs a few bitcoin-cli commands, then
shuts down all background processes before exiting.
(Actual API test suites have not been implemented yet.)

ApiTestConfig options can be used to skip tests and/or leave
background processes running indefinitely.
This gradle file is 'applied' by the main build file.

Usage:

  Run a full clean, build, download dao-setup.zip,
  and install the zip files contents in directory
  apitest/build/resources/main:

    ./gradlew clean build :apitest:installDaoSetup

  Download (if necessary) the dao-setup.zip file
  and install its contents in directory
  apitest/build/resources/main  (no build).

    ./gradlew :apitest:installDaoSetup
Unnecessary use of fully qualified name 'System.exit' due to existing
static import 'java.lang.System.exit'.  (line 100)

Avoid throwing raw exception types.  (lines 295, 302)
Avoid throwing raw exception types.

Combine nested if statements.
Avoid throwing null pointer exceptions.
@ghubstan ghubstan added this to In progress in Ship Bisq Daemon and API Jul 9, 2020
@wiz
Copy link
Member

wiz commented Jul 9, 2020

Does this require adding new jar deps?

@ghubstan
Copy link
Member Author

ghubstan commented Jul 10, 2020

Does this require adding new jar deps?

[EDITED 19-July-2020]

The org.junit.jupiter dependendency was upgraded to v5.6.2 -- only in the :apitest:test dependency definitions, nowhere else in the bisq main or test dependency definitions. The upgrade is necessary for running ordered tests.

build.gradle Outdated Show resolved Hide resolved
There is no need for this as any JavaFX based :desktop instances
will be started as background linux processes.
The `berkeleyDbLibPath` option now defaults to an empty string.
If not set to a berkeley lib path on the command line or the
apitest.properties file, this option is ignored, and 'bitcoind'
will be started without first exporting the berkeley db library
path.

In other words:  If the bitcoind binary is dynamically linked to
berkeley db libs, export the configured berkeley-db lib path before
starting 'bitcoind'.  If statically linked,  the berkeley db lib
path will not be exported.

Also fixed exception msgs to show missing config file's absolute path.
SetupTask submissions for Bisq background apps seednode, arbnode,
etc., would not always complete due to a blocking stderr stream
handler thread.join() call.  This change makes waiting on a bash
process err stream optional.
ApiTestMain will run all defined tests, but we also want to run
individual test suites and test cases, and they will need to
run the setup tasks as well.
Added :proto to the :apitest classpath for access to grpc
service stubs (to be) used in method (unit) tests.  Added new
GrpcStubs class to expose the grpc service stubs to method and
scenario tests.

The larger goal of :apitest is end to end testing, where :cli's
console output is checked for correctness.

This change partially addresses two other important use cases:

 * "method" testing -- an analog to unit testing
 * "scenario" testing -- an analog to narrow functional testing

For example, tests in the apitest.method package will directly
call grpc services, and asserts will be made on the return values
instead of console output.

Tests in the apitest.scenario package will check correctness
for broader use cases, such as funding a wallet, encrypting then
unlocking a wallet  for a specific time frame, or checking error
messages from the server when a "getbalance" call is made after
an "unlockwallet" timeout has expired.

The broader end to end tests will not use grpc stubs.
Avoid throwing raw exception types.

Document empty method body.
Codacy wants comments inside an empty method.
Add super class for all test types (method, scenario, end-to-end),
and an class & method level annotation for skipping tests.
These are method test cases for gRPC methods that have already been
well tested by :cli/test.sh
@ghubstan
Copy link
Member Author

Closing/reopening PR to force travis build after pricenode unit test failure .

@ghubstan ghubstan closed this Aug 18, 2020
Ship Bisq Daemon and API automation moved this from In progress to Done Aug 18, 2020
@ghubstan ghubstan reopened this Aug 18, 2020
Ship Bisq Daemon and API automation moved this from Done to In progress Aug 18, 2020
@ghubstan
Copy link
Member Author

Closing/reopening PR again to force codacy check after @ripcurlx made some codacy config changes.

@ghubstan ghubstan closed this Aug 19, 2020
Ship Bisq Daemon and API automation moved this from In progress to Done Aug 19, 2020
@ghubstan ghubstan reopened this Aug 19, 2020
Ship Bisq Daemon and API automation moved this from Done to In progress Aug 19, 2020
@ghubstan
Copy link
Member Author

Closing/reopening PR again to force codacy check...

Closing/reopening PR does not force codacy check.

A new commit was needed to force a codacy check after changes were
made to codacy rules.
This commit is for forcing a codacy check.  The previous
change to an .md doc did not force a codacy check.
Copy link
Member Author

@ghubstan ghubstan left a comment

Choose a reason for hiding this comment

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

RE 2ba0ee9: the private method was also moved to the bottom of the class.

This change did not force a codacy check.

sqrrm
sqrrm previously approved these changes Aug 19, 2020
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

ACK

Follow codacy rule against empty blocks.
@ghubstan
Copy link
Member Author

This PR should be merged before PR 4417.

@ripcurlx
Copy link
Member

@ghubstan I contacted Codacy on this issue. As the configuration problem should be solved (I've even deactivated it for testing), but it sill doesn't green light this PR.

@ghubstan
Copy link
Member Author

ghubstan commented Aug 24, 2020

@ripcurlx (& @sqrrm): ghubstan I contacted Codacy on this issue. As the configuration problem should be solved (I've even deactivated it for testing), but it sill doesn't green light this PR.

I pushed a change to shorten some too-long lines in one class, and codacy passed it. But I don't know if the problems are fixed yet, since codacy analyzed only the one changed class(?) I may push a small change to the next PR , based off this one, to see what codacy does with it.

By the way, the failed Travis CI JDK12 test failed to install JDK 12, so I won't close/re-open this PR to force another Travis build. I don't want to risk losing the Codacy Review PASS ;)

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

ACK

@sqrrm sqrrm merged commit d5c81ce into bisq-network:master Aug 24, 2020
Ship Bisq Daemon and API automation moved this from In progress to Done Aug 24, 2020
@ghubstan ghubstan deleted the draft-apitest branch September 16, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants