Rails 5 support patches #1492
Rails 5 support patches #1492
Conversation
fables-tales
force-pushed
the
rails-5-support-patches
branch
from
330b5bd to
cd82caf
Compare
|
This PR is green against specs, but not against our acceptance tests. |
| end | ||
|
|
||
| describe "matching a response" do | ||
| it "returns true for a response with a 3xx status code" do | ||
| any_3xx_code = 333 | ||
| any_3xx_code = 308 |
There was a problem hiding this comment.
Shouldn't these be dynamic depending on version? Or does 308 play ok for rails 4 etc too?
There was a problem hiding this comment.
308 is actually a valid code in the spec. 333 is not.
There was a problem hiding this comment.
The reason I made this change is that somewhere along the way, rails 5 no-longer accepts 3xx codes that aren't part of the spec in a bunch of places. 333 is not part of the spec. Given that all old versions of rails support any http response code that is in the spec, this seemed like the easiest change to make for this PR to work.
There was a problem hiding this comment.
This has been resolved
fables-tales
force-pushed
the
rails-5-support-patches
branch
from
cd82caf to
4cb18df
Compare
|
After a pairing session with @sgrif we're making some progress on this. We'll be spending a few more evenings on it to make progress. Things that definitely work at the moment:
Things that don't work:
|
|
Other things that don't work: My dotfiles. |
|
@sgrif and myself just saw this go green on rails master. Going to do my best to get it all-supported-versions compatible soon. |
fables-tales
force-pushed
the
rails-5-support-patches
branch
3 times, most recently
from
cdf6ca5 to
20bf0db
Compare
|
This is fully backwards compatible, rails 3, 4 and 5 support. Do not merge this yet. I have some rebasing/other work to do. |
|
Thanks @samphippen and @sgrif |
fables-tales
force-pushed
the
rails-5-support-patches
branch
from
d94ef14 to
d8916f1
Compare
This is done because rails 5 needs a newer version of rails console than will be installed by the implicit dependencies resolved via rubygems. It is also only added to the development group because we do not want webconsole to get in the way of any of the specs that we run.
We need the rails controller testing gem to be able to test the sample app. We also don't want it to run web console at all.
fables-tales
force-pushed
the
rails-5-support-patches
branch
from
d8916f1 to
10f0b89
Compare
|
@JonRowe @cupakromer I've just fixed up the history here. Would you mind taking a look through the commits and pointing out any changes that aren't made clear by the commit messages? |
|
Since beta1 is out, should this point at the released gems rather than git? |
|
@sgrif really, it should be beta1 and master. |
|
@samphippen My thinking here is while nothing should break before release, if it is we don't want your build failing do we? |
|
@sgrif yeah, I'm going to keep master in the allowed failures section, and add beta1 right now. |
|
👍 |
Rails 5 changes the class name generated for mailers from looking like `Thing` to looking like `ThingMailer`. Our cukes depend on this name, and so break as is. Here, I've added two tags, `@pre_rails_5` and `@post_rails_5`. The modification to the rakefile changes how we execute cukes based on the rails version, filtering to the correct cukes.
This now consistently calls `.to_i` on response codes, which ensures they're always an integer (accross rails versions). Additionally I replaced a number of `333` codes with `308`s. The reasoning for this is that rack no longer accepts non-spec 3xx codes in it's `redirect?` methods, which breaks our tests.
In Rails 5, the word "Listing" has been removed from the autogenerated scaffold header, so here we match with a simple regex.
Rails 5 now automatically appends the word `Mailer` to generated mailer classes. We now have to do that too.
This fixes a number of spec types, and requires the added setup lines. As a note, this is a soft, not hard dependency. We don't get automatic compatability. Instead, users will have to add the gem to their Gemfile if they want that functionality.
`controller.request.path_parameters` is no longer able to be directly assigned to, but should instead be mutated. Here, we take what we were doing before with the assignment and doing a reverse merge instead, which has the same affect.
We now have to mutate, instead of assigning, headers in the resp.headers property here.
fables-tales
force-pushed
the
rails-5-support-patches
branch
from
6f5f681 to
275905d
Compare
| @@ -56,6 +56,8 @@ matrix: | |||
| # Rails 5.x only supports 2.2 | |||
|
👍 |
|
@JonRowe I intentionally wasn't merging this unless we needed any further patches before rails went stable. Is it OK if I revert the merge and restore the branch? |
|
I guess but I'd rather people got this sooner rather than later, seems strange to have to get people on this branch and then keep rebasing simply because rails isn't released. |
|
it makes it easy to track all the changes for rails in one place, which is my concern. There also wasn't a changelog entry yet... |
|
I think on balance, having this merged is fine, actually. |
|
It's not the only place though (as I had initial work elsewhere) and makes it harder for others to contribute :) Happy for you to add a changelog direct to master |
|
If anyone trying rspec-rails with Rails 5 like me, you need these: gem "rspec-rails", git: "https://github.com/rspec/rspec-rails.git", branch: "master"
gem "rspec-core", git: "https://github.com/rspec/rspec-core.git", branch: "master"
gem "rspec-support", git: "https://github.com/rspec/rspec-support.git", branch: "master"
gem "rspec-expectations", git: "https://github.com/rspec/rspec-expectations.git", branch: "master"
gem "rspec-mocks", git: "https://github.com/rspec/rspec-mocks.git", branch: "master" |
|
Thanks @JuanitoFatas! I was getting all kinds of errors before this. |
|
Thanks for the tip, @JuanitoFatas! |
No description provided.