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 expectBody option and related tests #242

Merged
merged 3 commits into from Feb 1, 2020

Conversation

doesdev
Copy link
Contributor

@doesdev doesdev commented Jan 31, 2020

This is probably a niche need driven by my paranoia. However, for certain scenarios it adds peace of mind knowing that the server sent what I expected it to back to the client. While this isn't necessarily the place of a benchmarking tool, it is helpful to me to have this for stress testing sorts of purposes.

I understand if it doesn't fit. That being said it's mostly an unobtrusive addition. The only thing it really imposes is the addition of mismatches to the API results. I suppose I could have fenced that off so that it only shows in the results if the option is enabled. Let me know if that would be desirable and I can do that.

Copy link
Owner

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

I think this would be helpful in a few cases.

Can you make this throw if the requests option is set? These two will be uncompatible.

We might also want to add expectHeaders and expectStatusCode.

README.md Show resolved Hide resolved
@doesdev
Copy link
Contributor Author

doesdev commented Feb 1, 2020

Awesome, thanks for the quick feedback. I've added the error and related test as well as that warning. Good call on expectHeaders and expectStatusCode. Those will be nice to have as well. Will work on those over the next couple days here.

Do you think for the API results those 3 options should all share the mismatches count or should there be a prop for each of them? Was thinking it might be a good bit of noise to have 3 separate properties added to the results, but whichever way you think would be best.

Copy link
Owner

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina mcollina merged commit c16a0e5 into mcollina:master Feb 1, 2020
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.

None yet

2 participants