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

#1656: Update specs after upgrading rack-test to 0.7.0 #1658

Closed
wants to merge 3 commits into from

Conversation

ashkan18
Copy link

Rack test was ignoring empty arrays in its output which was fixed here. With these changes specs now show proper validation which doesn't match old ones anymore.

Updated rack-test to 0.7.0 and updated specs to match error messages we now receive with more details about missing params.

Fixes #1656

@grape-bot
Copy link

grape-bot commented Jul 13, 2017

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Jul 13, 2017

You have to re-generate appraisals, with appraisal generate --travis ; rubocop -a gemfiles/*.

CHANGELOG.md Outdated
@@ -3,6 +3,7 @@
#### Features

* Your contribution here.
* [#1658](https://github.com/ruby-grape/grape/pull/1658): Update `rack-test` to `0.7.0` and fix failing specs - [@ashkan18](https://github.com/ashkan18).
Copy link
Member

Choose a reason for hiding this comment

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

I would remove "and fix failing specs" cause that's not very interesting or useful.

@ashkan18
Copy link
Author

Looks like Rails 5 doesn't like this upgrade:

In Gemfile:
    rack-test (~> 0.7.0)
    rails (= 5.0.0) was resolved to 5.0.0, which depends on
      actionpack (= 5.0.0) was resolved to 5.0.0, which depends on
        rack-test (~> 0.6.3)

Will look into it later today.

@dblock
Copy link
Member

dblock commented Jul 13, 2017

I would imagine there's a newer Rails 5 that is ok with a later rack-test.

@dblock
Copy link
Member

dblock commented Jul 21, 2017

Bump @ashkan18

@ashkan18
Copy link
Author

Sorry for delay on this, I actually looked at different Rails V5 versions, rack-test is used by actionpack and it seems all of the versions are set to use

s.add_dependency "rack-test", "~> 0.6.3"

Not sure what the best way to proceed on this one now. I'm happy to close this PR till Rails upgrade.

@dblock
Copy link
Member

dblock commented Jul 21, 2017

I see. I think you should either open an issue in Rails or PR the upgrade in Rails and link it from here.

@laertispappas
Copy link

Shouldn't Rails_3.gemfile have rack-test v0.6.3 instead? Why should all Rails version have an updated version of rack test?

@dblock
Copy link
Member

dblock commented Aug 14, 2017

@laertispappas If you can make that work, great. I think because the behavior of the spec changes it's not that easy.

@dnesteryuk
Copy link
Member

This one might be closed, Grape already uses Rack-test 1.1.0

@dblock dblock closed this Jan 2, 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.

Upgrade to rack-test 0.7.0
5 participants