Skip to content

Conversation

@Nitrodist
Copy link
Contributor

There was a fix put in bc82276 (#296) that preserved the body of a
POST request that was being retried in the Retry middleware.
Unfortunately, there was no test added that ensures that this
functionality continues to work.

This commit adds the test to ensure that it continues to work in the
future.

@Nitrodist
Copy link
Contributor Author

Fails right now on 1.8.7 because public_send isn't a feature.

@Nitrodist
Copy link
Contributor Author

Updated to use send.

@Nitrodist
Copy link
Contributor Author

Can we merge this? :)

@kennethkalmer
Copy link

I had a quick look to help triage this, and it seems that rebasing this should not be a problem. I just wonder about changing @explode to @response_handler everywhere, it seems to only be a matter of taste. Since this PR I think one or two more tests got added, and they all stuck with @explode.

/cc @mislav @technoweenie @mike-bourgeous

@mislav
Copy link
Contributor

mislav commented Apr 9, 2015

Yeah, let's avoid the renames to keep the diff minimal.

@Nitrodist Nitrodist force-pushed the add-test-coverage-for-post-with-retry-middleware branch from f101933 to 66a8e2f Compare June 4, 2015 17:25
There was a fix put in bc82276 (lostisland#296) that preserved the body of a
POST request that was being retried in the Retry middleware.
Unfortunately, there was no test added that ensures that this
functionality continues to work.

This commit adds the test to ensure that it continues to work in the
future.
@Nitrodist Nitrodist force-pushed the add-test-coverage-for-post-with-retry-middleware branch from 66a8e2f to 45f5460 Compare June 4, 2015 17:30
@Nitrodist
Copy link
Contributor Author

1 failure now for a failing test in master, it seems. Can we merge now? :) cc @mislav @kennethkalmer

@mislav mislav closed this in 1316f4c Jun 12, 2015
@mislav
Copy link
Contributor

mislav commented Jun 12, 2015

Thanks! I've cleaned this up further.

@Nitrodist
Copy link
Contributor Author

👍

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.

3 participants