-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Test with rack-test 2.0 #2279
Test with rack-test 2.0 #2279
Conversation
Those two tests endpoint_spec are in the end testing internal details of rack-test. Change them so that they still test what was intended, but in a more robust way that does not break on changes on the implementation details of rack-test. Fixes: ruby-grape#2278
5951b8c
to
ab91474
Compare
Here's an example of a CHANGELOG.md entry: * [#2279](https://github.com/ruby-grape/grape/pull/2279): Test with rack-test 2.0 - [@terceiro](https://github.com/terceiro). Generated by 🚫 Danger |
@@ -432,7 +431,7 @@ def app | |||
end | |||
post '/upload', { file: '' }, 'CONTENT_TYPE' => 'multipart/form-data; boundary=foobar' | |||
expect(last_response.status).to eq(400) | |||
expect(last_response.body).to eq('empty message body supplied with multipart/form-data; boundary=foobar content-type') | |||
expect(last_response.body).not_to be_empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we losing something in this test? Is there something we can assert that ensures that the error is empty message body?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good question. TBH I could not figure out yet where this message in the body comes from. I thought it came from rack-test, but it doesn't. It seems that the new rack-test changes the way it composes requests with file uploads though.
Want to finish this @terceiro ? |
was this closed by mistake? I think we should get support for rack-test 2.0 at some point |
no, sorry for the noise. that MR does seem to address the issues here |
Those two tests endpoint_spec are in the end testing internal details of
rack-test. Change them so that they still test what was intended, but in
a more robust way that does not break on changes on the implementation
details of rack-test.
Fixes: #2278