Migrate contract tests to new API based on promises #19208
Conversation
ilmotta
requested review from
cammellos,
FFFra,
clauxx,
J-Son89 and
mohsen-ghafouri
ilmotta
force-pushed
the
ilmotta/rewrite-integration-tests-to-use-new-api
branch
from
a65b43f
to
d81a616
Compare
| :on-success action | ||
| :on-error on-error})) | ||
| (defn call-rpc | ||
| [method & args] |
There was a problem hiding this comment.
I could change the signature to become very similar to a synchronous function call because now there's no point in passing on-success and on-error callbacks.
| @@ -182,6 +182,29 @@ | |||
| (restore-fn) | |||
| (done)))))))) | |||
|
|
|||
| (defn test-app-initialization | |||
There was a problem hiding this comment.
I extracted these 2 tests here because they are identical for contract & integration tests.
Jenkins BuildsClick to see older builds (4)
|
| @@ -141,7 +140,7 @@ | |||
| (-> (wait-for [:messenger-started]) | |||
| (.then #(assert-messenger-started)))))) | |||
|
|
|||
| (defn integration-test | |||
| (defn test-async | |||
| _ (contract-utils/call-rpc | ||
| :accounts_saveAccount | ||
| (data-store/<-account (merge default-account {:emoji test-emoji}))) | ||
| accounts (contract-utils/call-rpc :accounts_getAccounts)] |
There was a problem hiding this comment.
hmm, I don't like the idea of making the rpc calls keywords if we're using strings in the application call instead. Should be similar imo. i.e: (contract-utils/call-rpc "accounts_getAccounts")
There was a problem hiding this comment.
I intend to change all usages of RPC calls to keywords @J-Son89, then it will be the opposite, it would be weird to use strings. Then have a function schema to reinforce that too. There's no reason to use strings because the value is immediately transformed to a string anyway to make the real RPC call. Feels more Clojurey to use keywords for unique values in the repo.
I've been using a keyword so far in the repo already. But I will change for this PR and do the changes in one go, no problem.
There was a problem hiding this comment.
All good to leave as is if you're going to change them anyway 👍
There was a problem hiding this comment.
too quick on the draw :) - I'm fine with changing them to keywords provided the intention is to do the same for the other uses in the app 👌
There was a problem hiding this comment.
Noo really you're correct. This kind of change should be consistently applied in the repo. I put the cart before the horse 🤷🏼
ilmotta
force-pushed
the
ilmotta/rewrite-integration-tests-to-use-new-api
branch
from
2ceece5
to
5cbfbda
Compare
|
Amazing! So elegant!! |
Summary
This PR changes contract test code to use the new API previously implemented for integration tests. Both types of test share similar characteristics and thus, can use the same API.
I have also renamed the utility function
h/integration-testtoh/async-testbecause it's a general solution used now for both contract & integration tests.Steps to test
make test-contractstatus: ready