fix for rspec #367
fix for rspec #367
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
| @@ -83,6 +83,10 @@ class Railtie < ::Rails::Railtie | |||
| React::ServerRendering.reset_pool | |||
| # Reload renderers in dev when files change | |||
| ActionDispatch::Reloader.to_prepare { React::ServerRendering.reset_pool } | |||
| if defined? RSpec::Rails | |||
| require 'react/rails/test_helper' | |||
There was a problem hiding this comment.
Would it hurt to always require this file? It's not Rspec-specific, right? So we could move it to react/rails.rb along with the other require calls.
Also, if this helper is specific to Rspec, what do you think about calling it RspecHelper instead of TestHelper? I think that expresses its purpose a little bit more.
There was a problem hiding this comment.
Or, maybe ViewTestHelper if this also works with Minitest
|
Thanks for adding this fix! Could you also add a test for this behavior? I see that you were using Rspec, but this project uses Minitest. Can you test this behavior by adding an |
|
I have no idea to test it without rspec... |
|
Yes, that would be fine! |
|
add rspec tests |
|
Hmm, not sure what's wrong with this build. It turns out this problem is part of a bigger issue. I was able to reproduce the issue using Do you want to try that branch? If it works for you, I'll merge it, since I think it also fixes the issue described here: #353 (comment) |
in view tests of rspec, we must assign vars by myself.
|
I think it's just a travis's problem... |
|
Sorry @masarakki , I think this problem was larger than just Rspec tests. It was also broken for:
So, I refactored the helper a bit and used I prefer that option because it's less code overall and it handles more of those broken cases. Could you check that it works for you, too? I've just released it in version |
|
v1.3.3 works well. great job!! |
in view tests of rspec, we must assign vars by myself.