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

REST helper update, fix asynchronicity to get response from APIs #504

Merged
merged 3 commits into from Jun 20, 2017

Conversation

Projects
None yet
2 participants
@atrevino
Contributor

atrevino commented May 5, 2017

No description provided.

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik May 10, 2017

Member

Thanks for the update. However I don't like that there is no easy way to create request and you need to use unirest API to do that. This breaks CodeceptJS principles.

What I'd like to see is sending request like

I.sendPost('/users', { name: 'davert' }, { headers: { auth: 11111111 }, cookies: {} });

or set headers for all requests:

I.haveRequestHeader({ auth: 111111 });
I.sendPost('/users', { name: 'davert' });

so the idea is that most of requests to be available via simple sendGet, sendPost, etc. So you won't need method chaining and so on. Sure there should be an option to start request manually with generateRequest but this should not be the default option

Member

DavertMik commented May 10, 2017

Thanks for the update. However I don't like that there is no easy way to create request and you need to use unirest API to do that. This breaks CodeceptJS principles.

What I'd like to see is sending request like

I.sendPost('/users', { name: 'davert' }, { headers: { auth: 11111111 }, cookies: {} });

or set headers for all requests:

I.haveRequestHeader({ auth: 111111 });
I.sendPost('/users', { name: 'davert' });

so the idea is that most of requests to be available via simple sendGet, sendPost, etc. So you won't need method chaining and so on. Sure there should be an option to start request manually with generateRequest but this should not be the default option

@atrevino

This comment has been minimized.

Show comment
Hide comment
@atrevino

atrevino May 16, 2017

Contributor

hey @DavertMik
You want a specific method to return the response from API call?
Like:

I.haveRequestHeaders({auth:111});
I.sendPost('/users');

let response = yield I.getResponse();

or directly from I.sendPost('/user');
Like

I.haveRequestHeaders({auth:111});
let response = yield I.sendPost('/users');
Contributor

atrevino commented May 16, 2017

hey @DavertMik
You want a specific method to return the response from API call?
Like:

I.haveRequestHeaders({auth:111});
I.sendPost('/users');

let response = yield I.getResponse();

or directly from I.sendPost('/user');
Like

I.haveRequestHeaders({auth:111});
let response = yield I.sendPost('/users');
@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik May 17, 2017

Member

Yes, I like this a bit more

I.haveRequestHeaders({auth:111});
let response = yield I.sendPost('/users');

Thanks :)

Member

DavertMik commented May 17, 2017

Yes, I like this a bit more

I.haveRequestHeaders({auth:111});
let response = yield I.sendPost('/users');

Thanks :)

@atrevino

This comment has been minimized.

Show comment
Hide comment
@atrevino

atrevino Jun 1, 2017

Contributor

Sorry for the delay, I'm doing tests about this. Probably will push only for review.

Contributor

atrevino commented Jun 1, 2017

Sorry for the delay, I'm doing tests about this. Probably will push only for review.

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Jun 2, 2017

Member

Cool. Waiting for the update!

Member

DavertMik commented Jun 2, 2017

Cool. Waiting for the update!

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Jun 20, 2017

Member

Thanks. I will work on improve it and maybe add some tests

Member

DavertMik commented Jun 20, 2017

Thanks. I will work on improve it and maybe add some tests

@DavertMik DavertMik merged commit fb32e70 into Codeception:master Jun 20, 2017

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment