There are a number of use cases where a user may want to see a single
offer instead of every offer for a currency pair on the buy or sell side.
The changes are:
- Add getoffer to grpc.proto
- Add new method to GrpcOffersService, CoreApi, CoreOffersService
- Add new method to CLI
- Adjust create offer tests to use this new convenience
Revert Task.java back to the state it was in prior to
my attempts to make it work for passing error msgs to a
gRPC CLI running 'placeoffer'. Adjust the affected test
accordingly.
Separates offer placement from offer creation to fix tx result
handling problem in GrpcOffersService, and readies the core api
for a new CLI 'placeoffer' implementation. Offer placement still
happens in the api's 'createoffer', but we may want to change it
to show the created offer to a CLI user for review, prior to manual
placement via a new 'placeoffer offer-id' (of 'confirmoffer offer-id'?)
api method.
The CLI needs to be able to register a REFUND_AGENT using the
'refund_agent' or 'refundagent' parameter value (in any case),
so an alt-name mapping was added to the enum def.
The CreateOfferRequest's price field type was changed from long to string,
so a CLI param like 10000.0055 can be passed to the server as is, to be
scaled and converted there. In general, we want to keep validation logic
on the CLI as simple as possible, and use existing core logic to
validate args and convert arg types when needed.
The string constants deleted from the test case are re-defined as enums,
but the test harness still passes around strings (enum.name()) because
the handling of invalid dispute agent type string args needs to be
tested. (Reminder: CLI does not accept any enum arguments.)
Bats version check tests now use a bash script for parsing
the value from the Bisq class file, and these test cases
no longer need to be manually updated.
This commit contains most of the changes suggested by @chimp1984 in
his api-suggestions branch. See
961703ecea
A new 'registerdisputeagent' method was also added to MainCli, finishing
the work to support registration of mediators and refund agents on
arbitration daemons running in regtest mode. This method cannot be
used to register dispute agents on mainnet; users will see an error
msg if they try.
A unique hard coded debug listening port is assigned to the different
Bisq app types in the BisqAppConfig enum, and background Bisq apps
will be started with remote debug options if the scaffold-setup method
is passed an --enableBisqDebugging=true option.
* Added enableBisqDebugging (default=false) option to ApiTestConfig.
* Added remoteDebugPort field to BisqAppConfig enum.
* Added debugOpts field to BisqApp (using BisqAppConfig#remoteDebugPort).
* Appends debugOpts to exported JAVA_OPTS environment variable if present.
* Removed messy quotes from BisqAppConfig enum javaOpts values.
* Removed redundant return statement from BisqApp#shutdown().
This change removes three options: runArbNodeAsDesktop,
runAliceNodeAsDesktop, and runBobNodeAsDesktop, which should have
been deleted when the supportingApps option was added. The comma
delimited list of apps passed with the supportingApps option
now determines whether arbitration / bob / alice nodes are started as
desktops or daemons.
The :apitest GrpcStubs class was removed and recreated in the :cli
subproject, to be used by both :cli and :apitest. CliMain was changed
to use the new GrpcStubs.
* The bats test script was moved to the apitest subproject and renamed.
* Version tests were updated for release 1.3.7.
* The duplicated "test getoffers buy eur check return status" was
replaced by a new "test getoffers sell eur check return status" test.
* The bats dependency was switched to bats-core because development
has halted on https://github.com/sstephenson/bats/tree/master.
The new bats repository is
https://github.com/bats-core/bats-core/tree/master
Adds the missing String[] params to the method signature, so test cases
can pass any needed combination of options to the scaffolding setup
from a @BeforeAll method.
This commit changes the access modifier on
bisq.common.config.CompositeOptionSet to make
it visible to bisq.apitest.config.ApiTestConfig,
and the duplicate bisq.apitest.config.ApiTestConfig
was deleted from 'apitest'.
The default bitcoind / bitcoin-cli rpcport option has been changed
from 18443 to 19443, to help avoid rpcport conflicts between apitest's
bitcoind instances and other bitcoind and/or bitcion-qt instances
which are probably using the bitcoin-core default (regtest) rpcport
18443.
However, this commit cannot include other changes for avoiding bind
address:port conflicts between apitest bitcoind instances and other
regtest bitcoin-core instances because bitcoinj's bind port is hardcoded
in RegTestParams.java as 18444.
In order to avoid bitcoin-core regtest mode bind address conflicts,
you must start or restart your bitcoind or bitcoin-qt instance with a
non-default bind port argument, e.g.
bitcoin-qt -regtest -port=20444
This commit adds a -fallbackfee=0.0002 parameter to the start 'bitcoind'
command to keep bitcoin-core v0.20.1 working as v0.19.1 does -- with the
'fallbackfee' enabled.
Bitcoin v0.20.0 contains a fix for inconsistent behaviour related
to this fallbackfee configuration. Prior to v0.20, fallbackfee
was disabled (0) by default for the mainnet chain, but enabled
(0.0002) for the testnet and regtest chains.
A test case with bitcoin-core v0.20.1 was breaking on
the bitcoin-cli 'sendtoaddress' command, which was returning an
error message instead of a tx-id:
error code: -4
error message:
Fee estimation failed. Fallbackfee is disabled. \
Wait a few blocks or enable -fallbackfee.
Bitcoin-core v0.20.0 release notes contain info about this change:
https://bitcoin.org/en/release/v0.20.0#updated-rpcs-1
The Bitcoin-core PR is https://github.com/bitcoin/bitcoin/pull/16524
The Scaffold#tearDown() method was split into two methods. The
original tearDown() now passes the background process/task array
to a new shutDownAll() method. This new method loops through the
tasks in a more readable way, plainly expressing the intent to log
all shutdown exceptions for each process being shut down, but not
throwing an exception while processes are being shut down.
The new shutDownAll() method returns the first shutdown exception
encountered, which in turn is passed up to the test case's @AfterAll
method.
The test harness should not fail a test case's @AfterAll (teardown)
method on the first background instance shutdown exception. This
change makes the shutdown logic similar to the startup's: it caches
any exceptions that may have occurred during an instance shutdown,
logs them, then proceeds to shut down the next background instance.
An IllegalStateException (the 1st one) is passed up to @AfterAll method
only after the scaffolding teardown process is complete, to avoid leaving
any orphaned java or bitcoind processes running after a java system
exit.
The Scaffold set up was calling System.exit(1) when it encountered a
configuration error, or a problem starting bitcoind & bisq instances.
This incorrect error handling was hiding errors from gradle, and
preventing tests that would otherwise successfully complete.
This change fixes the problem by throwing an IllegalStateException up
to the test case's @BeforeClass method -- to be caught and used in a
JUnit fail(ex) call. An 'initializationError' triggered in @BeforeClass
correctly bubbles up to gradle, and will not block execution of remaining
tests. A gradle Test Summary containing any initialization errors is also
produced in <project-dir>/apitest/build/reports/tests/test/index.html
This change also fixes many import statements and asserts to ensure
'org.junit.jupiter.api.*' is used in place of 'org.junit.*', for
proper gradle integration.
Some explanation is needed about why Bob & Alice have non-zero
initial BTC and BSQ balances when tests run. The comments
also include links to more detailed information about the DAO/
regtest testing environment.
This change reorganizes the ':apitest' subproject to conform to a
typical gradle project, where all JUnit test cases are located in
the subproject's test sources folder. This makes running tests
from an IDE or gradle command line interface work as expected.
It will also help keep Travis CI configuration simple.
To avoid interfering in normal builds, the gradle ':apitest test' task
is disable by default; API tests will only run when a '-Dforce-true'
system property is passed to gradle.
To run API tests, run a normal build and install dao-setup files:
./gradlew clean build :apitest:installDaoSetup
Then run the tests:
./gradlew :apitest:test -Dforce=true
Try to avoid adding the '-Dforce=true' property to any other gradle
tasks, because this enables the ':apitest test' task, and would kick
off API tests before a normal build completed.
The build.gradle file was modified to support this code reorg, and
the 'org.junit.jupiter' dependendency was upgraded to v5.6.2 -- only
in the ':apitest:test' dependency definiitions, not anywhere else in
the bisq dependency definitions. The upgrade is necessary for
running ordered tests.
Since the scaffolding may be set up from either test cases (under the
test src folder), or a class under the main src folder, some changes
were made to ensure configuration paths are correct for either use
case. For example, when the 'bisq-apitest' script is run from the root
project directory, the current working directory is the root project
directory. When gradle or an IDE is used to run @Test cases, the
current working directory is :apitest subproject directory.
The main source's ApiTestMain class has been stripped down, and exists
only to keep the gradle build happy -- it needs a 'mainClassName'
property. But this main driver does have uses. See the class comments.
The other changes in this commit were made to fix style and syntax
problems.
JUnit tests are being moved to the subproject's test sources
directory for the sake of convention -- for developers using
this test harness, and for IDE and gradle JUnit integration.
This change removes code for installing a regtest bitcoin.conf file.
It also removes an unused bitcoin.conf file from the main resources
directory.
Now, the bitcoind startup command passes all configurations on the
command line. (See bisq.apitest.linux.BitcoinDaemon.java)
This change makes configurable the amount of time (ms) each Bisq
instance is given to initialize before starting (a) another Bisq
instance, or (b) tests.
Some of these 'method' tests make more than one gRPC call, but
they are for checking correctness of a single gRPC method,
and don't quite fall into the 'scenario' test category.
Also added a new base ScenarioTest class, which extends
MethodTest; Method and Scenario tests cases have access to gRPC
service stubs, but end to end test cases should never use them.
This change replaces the non-intuitive numSetupTasks=<Integer>
configuration option with a supportingApps=<String> option. It
accepts a comma delimited list of app names, and determines
which background apps will be started in @BeforeClass.
None of the current method tests need all supporting apps, and
this change will reduce scaffolding setup and teardown time.
The current method test cases were changed to use this option.
WalletProtectionTest and GetVersionTest only need to start "alicedaemon".
GetBalanceTest needs "bitcoind,seednode,arbdaemon,alicedaemon".
API test cases are not in a maven/gradle project test folder.
IDEs may not automatically configure JUnit test launchers,
and a gradle build command will not automatically run tests.
This class is provided as a convenience until gradle tasks
for running test cases are implemented.
These @Order-ed test cases should be run with OrderedRunner.
They are called 'method' tests because they are the closest
thing to an API 'unit' test. Method tests are not end to end
tests; they have access to gRPC service stubs and do asserts
on Java values. We can write 'method' tests for the new
gRPC methods we need to implement (test driven development).
Some notes about the three API test categories...
The plan is to categorize some of the narrower functional tests
as 'scenario' tests. Like 'method' tests, 'scenario' tests will
have access to gRPC service stubs and do asserts on Java values,
but they will cover use cases requiring several gRPC and bitcoin-cli
method calls. One example is funding a wallet: using bitcoin-cli
and bisq-cli to generate new addresses, send BTC from bitcoin-core
to a Bisq wallet, generate a bitcoin block, and check the Bisq
wallet's balance. Another example is wallet unlocking/locking
scenarios that are a bit more complex than 'unit' or 'method' tests.
The third category of tests will be end to end, and live in an
'e2e' package. They will cover more complex use cases and
do asserts on the Bisq CLI's console output (stdout/stderr),
not return values from gRPC service stubs.
There may sometimes be a fine line between what is a 'scenario'
test and an 'e2e' test. But what starts out as a 'scenario' test
during test driven development can migrate to an 'e2e' test after
thorough testing in the quicker 'scenario' dev/test cycle.
Although JUnit is geared towards running randomly ordered unit
tests, we can use it to write API tests to be run in a pre-defined
order. This is necessary because API tests will be functional
(narrow to broad) and end-to-end; we want to be able to write
@Test methods that run in @Order.
The next commit will add a JUnit OrderedRunner class, and an @Order
annotation.
Many test cases will need to run on a clean, initialized dao-setup;
we do not want call dao-setup.gradle tasks from Java.
For now, every JUnit test case will refresh the dao-setup data in
@BeforeClass, but @AfterClass will not clean out the dao-setup files.
We want to be able to look at bitcoin-core and bisq app data & log
files after a test.
Given the difficulty of refreshing dao-setup files using Java,
I took a short cut and used the apitest.linux pkg's BashCommand
to replace dao-setup files.
Some bitcoin.conf and blocknotify configuratation logic was
also moved from ApiTestConfig to Scaffold in this commit.
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.
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.
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.
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.
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)
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
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.
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.
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.
