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

Refactor apitest for api calls to any daemon type #4525

Merged
merged 9 commits into from
Sep 16, 2020

Conversation

ghubstan
Copy link
Contributor

These changes fix the test harness so test api calls can be made to any type of daemon (arbitrator, bob, alice), not just alice. A new DisputeAgentRegistrationTest case is included in these changes -- it needs to send requests to an arbitration daemon.

This PR should be reviewed & merged after 4524.

This change supports registering mediators and refund agents on
daemons running on regest or testnet chains.  Registering
arbitrators is not supported.
Most test cases send requests to the alicedaemon, but new test cases
will need to be able to send requests to arbitration and bob daemons.
This test case checks that mediators and refund agents can be
registered over grpc, but not on mainnet.
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

Looks ok, but relying on strings for the tests seems fragile as noticed.

createRegisterDisputeAgentRequest("arbitrator");
Throwable exception = assertThrows(StatusRuntimeException.class, () ->
grpcStubs(arbdaemon).disputeAgentsService.registerDisputeAgent(req));
assertEquals("INVALID_ARGUMENT: arbitrators must be registered in a Bisq UI",
Copy link
Member

Choose a reason for hiding this comment

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

Relying on string matching for tests makes them fragile. At a minimum we should probably add a note for strings that are used in this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix this in another PR.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think this happens quite a bit in other tests as well iirc. Not urgent, but good to have a better solution for the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This string matching problem has bitten me a few times, when I misspelled "bitciond" in some test case setups.
PR 4540 fixes it.

@sqrrm sqrrm merged commit 6cf10c4 into bisq-network:master Sep 16, 2020
@ghubstan ghubstan deleted the 2-refactor-apitest-grpcstubs branch September 19, 2020 19:03
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.

2 participants