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 LongRunningTradesTest to apitest #5303

Merged
merged 17 commits into from
Mar 19, 2021

Conversation

ghubstan
Copy link
Contributor

This new api testcase can run a long series of regtest trades by looping over modified TakeBuyBTCOfferTest and TakeSellBTCOfferTest cases. The purpose is to help reproduce problems and isolate bugs Bisq's core and api layers.

LongRunningTradesTest is not enabled by default; it will not run in a default test environment (such a Travis CI). Enviornment variable LONG_RUNNING_TRADES_TEST_ENABLED must exist for the test to run. The env variable can be set in a bash shell before running the test case in a shell (using a gradle command), or the environment variable can be set in an Intellij test launcher's Environment variables field.

The modifed (short running) TakeBuyBTCOfferTest and TakeSellBTCOfferTest cases run as before.

Changes include:

  • Add static boolean isLongRunningTest to AbstractOfferTest.

  • Add looping control Supplier maxTradeStateAndPhaseChecks to AbstractTradeTest. It uses the isLongRunningTest flag in the superclass to help define the wait times for trade state/phase changes during short and long running tests.

  • Made AbstractTradeTest assert(true, trade.isDepositPublished) conditional upon isLongRunningTest value. Long running trade test asserts have to be looser due to increasing latency of wallet, offer and trade operations in the server as the trade counts increase.

  • Overload ApiTestCase#startSupportingApps with additional flag: startSupportingAppsInDebugMode. Default is false. This makes starting background apps in debug mode a bit more convenient and self explanatory.

PR #5302 should be reviewed/merged before this one.

- Add PRICE_CHECK_FAILED to enum AvailabilityResult.
- Add enum AvailabilityResult to TakeOfferReply message.
- GrpcErrorMessageHandler  A new ErrorMessageHandler implementation
  to get around the api specific problem of having to use Task
  ErrorMessageHandlers that build task error messages for the UI.

- GrpcExceptionHandler  A new method for working with the
  ErrorMessageHandler interface.

- GrpcTradesService, CoreApi, CoreTradesService:  Ajdusted takeoffer
  error handling to give a failure reason provided by the new
  GrpcErrorMessageHandler.
Add debugging convenience to core.api.model.TxInfo.
This new api testcase can run long series' of regtest trades by looping
over modified TakeBuyBTCOfferTest and TakeSellBTCOfferTest cases.  The
purpose is to help reproduce problems and isolate bugs Bisq's core and
api layers.

LongRunningTradesTest is not enabled by default;  it will not run in a
default test environment (such a Travis CI).  Enviornment variable
LONG_RUNNING_TRADES_TEST_ENABLED must exist for the test to run.
The env variable can be set in a bash shell before running the test
case in a shell (using a gradle command), or the environment variable can
be set in an Intellij test launcher's Evironment variables field.

The modifed (short running) TakeBuyBTCOfferTest and TakeSellBTCOfferTest
cases run as before.

Changes include:

- Add static boolean isLongRunningTest to AbstractOfferTest.

- Add looping control Supplier maxTradeStateAndPhaseChecks to AbstractTradeTest.
  It uses the isLongRunningTest flag in the superclass to help define the
  wait times for trade state/phase changes during short and long running tests.

- Made AbstractTradeTest assert(true, trade.isDepositPublished) conditional upon
  isLongRunningTest value.  Long running trade test asserts have to be looser due
  to increasing latency of wallet, offer and trade operations in the server as the
  trade counts increase.

- Overload ApiTestCase#startSupportingApps with additional flag:
  startSupportingAppsInDebugMode. Default is false.  This makes
  starting background apps in debug mode a bit more convenient
  and self explanatory.
@ripcurlx ripcurlx added this to the v1.6.0 milestone Mar 15, 2021
@ripcurlx ripcurlx requested a review from sqrrm March 15, 2021 09:01
- Add description msg TakeOfferReply proto, and fromProto method
  to core.offer.enum AvailabilityResult.  The description field
  maps a client usable error message to the enum.

- Adjust GrpcErrorMessageHandler to add AvailabilityResult.description()
  to takeoffer reply.

- Refactor (split up) GrpcClient's takeOffer.  Add getTakeOfferReply()
  to give clients a chance to make choices based on the reply's
  AvailabilityResult when the takeoffer command did not result in a
  trade. (Some errors are fatal, some not.)
@ghubstan
Copy link
Contributor Author

I hope some of the other devs will help resolve this issue.

To run the test case, clone this branch and run LongRunningTradesTest

  • Build: $ ./gradlew build :apitest:installDaoSetup -x test

  • Create an Intellij LongRunningTradesTest test case launcher with
    VM options: -DrunApiTests=true
    Environment variables: LONG_RUNNING_TRADES_TEST_ENABLED=true

This change uses recently added walletService.isAddressUnused to ensure
the api's CoreWalletsService creates an unused address if none exists.

- grpc.proto:  Add bool isAddressUnused field to message AddressBalanceInfo.
- AddressBalanceInfo:  Adjust AddressBalanceInfo proto wrapper.
- CoreWalletsService:  Use walletService.isAddressUnused in getFundingAddresses.
- GrpcClient:  Adjust to modified AddressBalanceInfo.
- TableFormat, ColumnHeaderConstants:  Add 'Is Used' column to getfundingaddresses output.

Note: bugfix is out of scope for this PR, but the test case helped expose this bug.
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.

utACK

@sqrrm sqrrm merged commit 53f42c4 into bisq-network:master Mar 19, 2021
@ghubstan ghubstan deleted the 03-add-long-running-trade-test branch March 19, 2021 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants