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

Adding RSpec 3 and rails-api Gem Support #5

Merged
merged 8 commits into from
Jul 2, 2014
Merged

Adding RSpec 3 and rails-api Gem Support #5

merged 8 commits into from
Jul 2, 2014

Conversation

groomsy
Copy link
Contributor

@groomsy groomsy commented Jun 28, 2014

Notice one of my comments on a hack I had to do for file uploads (using ActionDispatch::Http::UploadedFile and rendering file objects when displaying the logs). Feel free to take any or all of it. If you come up with a more sophisticated way of displaying the file objects, please let me know.

@lserman
Copy link
Contributor

lserman commented Jul 1, 2014

Thanks Todd, it looks pretty good. I want to test it on some of our apps before I merge, probably get to that later in the week.

@groomsy
Copy link
Contributor Author

groomsy commented Jul 1, 2014

No worries. If you think of something to do about the file stream thing, let me know. My solution was less than ideal. Let me know if you have any questions.

Sent from my iPhone

On Jun 30, 2014, at 8:05 PM, Logan Serman notifications@github.com wrote:

Thanks Todd, it looks pretty good. I want to test it on some of our apps before I merge, probably get to that later in the week.


Reply to this email directly or view it on GitHub.

Also passing example back into Middleware, but setting it up in the after hook in rspec.
@groomsy
Copy link
Contributor Author

groomsy commented Jul 1, 2014

@lserman Let me know if I missed anything. I believe I did everything you suggested. Thanks for catching the need for those checks. I was pretty heads down and only cared that it was working for my needs, so I apologize for not checking.

@lserman
Copy link
Contributor

lserman commented Jul 1, 2014

I made a mistake, can you change to

config.after(:each) do |ex|
  _example = example || ex
  # ...
    Avocado::Middleware.invoke(_example, request, response)

Due to the way Ruby parses local vars using example ||= ex will not work in this case. After that can you verify it's still working in RSpec 3 and with your rails-api app? Then it will be ready for merge 👍

@groomsy
Copy link
Contributor Author

groomsy commented Jul 2, 2014

Boy, my face is red.

Okay: My previous commit of

if defined?(ActionController::API)
  config.before(:suite) { ActionController::API.send :include, Avocado::Controller }
end

was not right. I needed to burry the conditional in the braces instead. So I made that change (I just realized that I wasn't getting documentation generated).

I also had to slightly modify the statement for setting _example. I was getting some trouble from calling example in RSpec 3.

Ensure this works for you (I've double and triple checked it on my end).

lserman added a commit that referenced this pull request Jul 2, 2014
Adding RSpec 3 and rails-api Gem Support
@lserman lserman merged commit 3087f03 into metova:master Jul 2, 2014
@lserman
Copy link
Contributor

lserman commented Jul 2, 2014

Made a couple changes to fix #6 - all files should now be replaced by text so I removed the hardcoded params[:data] you had in there. Lemme know if you're still seeing problems with that or in general. 👍

@groomsy groomsy deleted the feature/RSpec3APIGemSupport branch July 2, 2014 19:41
@groomsy groomsy restored the feature/RSpec3APIGemSupport branch July 2, 2014 19:50
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.

2 participants