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

Port the Request module from the integration framework minus the typed client (only allow arbitrary raw JSON requests) #56

Closed
piotr-iohk opened this issue Mar 13, 2019 · 3 comments
Assignees

Comments

@piotr-iohk
Copy link
Contributor

piotr-iohk commented Mar 13, 2019

Context

Since we are starting to shape API for the wallet we want a way to have it tested using integration tests.
This task is about porting the Request module from the cardano-wallet-legacy into new wallet.

For the new wallet's integration tests we want to use only the unsafeRequest approach and give up on using typed-client approach as the first one turned out to be more versatile in terms of producing both positive and negative test scenarios in the past.

Decision

Port all the functions from Request into new wallet that are necessary to submit raw JSON request (unsafeRequest) and get raw response from the request. The response needs to contain response body and response code such that it can be than asserted using test framework's DSL (#55).
Do not port anything related with the typed-client approach.

Acceptance Criteria

  1. One must be able to submit a request using raw-JSON as a parameter if needed (unsafeRequest approach)
  2. One must be able to get the response from the API (body and response code)

Development Plan

  • Copy Request.hs from old codebase
  • Keep unsafeRequest and remove servant-client specific code.
  • If there is a HttpException, return that to caller in Either type.
  • Add a test case to check that unsafeRequest works.
  • Call unsafeRequest just request because we don't have typed requests.

PR

Number Base
#83 master
#104 master

QA

The initial test should be run with:

stack test cardano-wallet:integration

This can be considered to be working if the request function can be used for new test scenarios.

@KtorZ KtorZ mentioned this issue Mar 14, 2019
7 tasks
@KtorZ KtorZ added this to the Wallet Layer Integration milestone Mar 14, 2019
@rvl rvl self-assigned this Mar 18, 2019
rvl added a commit that referenced this issue Mar 19, 2019
This provides `unsafeRequest`. A little bit of the DSL module was
ported so that I could test the function.

Relates to #56
rvl added a commit that referenced this issue Mar 19, 2019
This provides `unsafeRequest`. A little bit of the DSL module was
ported so that I could test the function.

Relates to #56
rvl added a commit that referenced this issue Mar 20, 2019
This provides `unsafeRequest`, renamed to `request`, plus some
variations.

A little bit of the DSL module and the Scenario module was ported so
that I could test the function.

Relates to #55 and #56
rvl added a commit that referenced this issue Mar 20, 2019
This provides `unsafeRequest`, renamed to `request`, plus some
variations.

A little bit of the DSL module and the Scenario module was ported so
that I could test the function.

Relates to #55 and #56
rvl added a commit that referenced this issue Mar 20, 2019
This provides `unsafeRequest`, renamed to `request`, plus some
variations.

A little bit of the DSL module and the Scenario module was ported so
that I could test the function.

Relates to #55 and #56
rvl added a commit that referenced this issue Mar 20, 2019
This provides `unsafeRequest`, renamed to `request`, plus some
variations.

A little bit of the DSL module and the Scenario module was ported so
that I could test the function.

Relates to #55 and #56
rvl added a commit that referenced this issue Mar 20, 2019
This provides `unsafeRequest`, renamed to `request`, plus some
variations.

A little bit of the DSL module and the Scenario module was ported so
that I could test the function.

Relates to #55 and #56
@piotr-iohk
Copy link
Contributor Author

lgtm 👍

I have added also response code verification in #104.
Also, I have some ideas to extend the framework to make it more negative scenarios friendly. Will add this also to wiki:

  • ability to get a response code from the response and use DSL expectiations to assert if it is as expected (kinda addressed in Add response code verification into integration DSL #104)
  • ability to explicitly set request headers in order to verify destructive scenarios
  • ability to construct and send non-valid JSON request body in order to test destructive scenarios

@KtorZ
Copy link
Member

KtorZ commented Mar 22, 2019

@piotr-iohk Are you intending to the development for the points you mentioned as part of this ticket? (in which case, I'd suggest to put it back to 'In Progress'), or can close this one and tackle the remaining points as debts next week ?

@piotr-iohk
Copy link
Contributor Author

piotr-iohk commented Mar 22, 2019

@KtorZ - yes, the latter 👍 i.e.:

close this one and tackle the remaining points as debts next week..

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

No branches or pull requests

3 participants