-
-
Notifications
You must be signed in to change notification settings - Fork 630
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
VERSION 9.0: Fixing rails 3.2 test run #926
Conversation
hi @justin808 i fixed 3.2 but run into Server Rendering spec failing
in log/test.log is there some ReactOnRails configuration changes missing in dummy_no_webpacker or it is actual issue? |
@morozovm We need to make sure that lines like this: are never executed if the Webpacker Gem is not installed... In most cases, we can call I'm not sure what to do on the rescue statement, other than maybe when the app loads if Webpacker is not defined, then we'll create the I think probably the latter is simpler. |
Reviewed 3 of 3 files at r1. rakelib/run_rspec.rake, line 25 at r1 (raw file):
Nice job on adding the webpacker tag. CC: @alexfedoseev spec/react_on_rails/spec_helper.rb, line 30 at r1 (raw file):
Why was this happening, @morozovm? spec/react_on_rails/utils_spec.rb, line 17 at r1 (raw file):
or
Will either of those work? I typically do the latter. Comments from Reviewable |
spec/react_on_rails/utils_spec.rb, line 17 at r1 (raw file): Previously, justin808 (Justin Gordon) wrote…
fixed Comments from Reviewable |
@justin808 Comments from Reviewable |
spec/react_on_rails/spec_helper.rb, line 30 at r1 (raw file): Previously, justin808 (Justin Gordon) wrote…
activesupport 3.2.22.5 demands test-unit to be loaded
this hack is solution to loading test-unit and rspec simultaneously by tricking test-unit in into thinking it is already done running. As far as i can see it is only viable solution. Comments from Reviewable |
rakelib/run_rspec.rake, line 25 at r1 (raw file): Previously, justin808 (Justin Gordon) wrote…
🙏 Comments from Reviewable |
@morozovm Please rebase your changes on top of master and when github says I can merge, I will. |
Reviewed 4 of 4 files at r2. lib/react_on_rails/utils.rb, line 77 at r2 (raw file):
@morozovm did you try this thoroughly and provide tests for the cases:
I created a small test, and require "active_support/all"
def start_summer
raise "overheated!"
end
begin
start_summer
rescue ActiveSupport::Inflector.safe_constantize("Xyz")
puts "Let me tell you about heat! #{e.inspect}"
end output:
spec/react_on_rails/spec_helper.rb, line 30 at r1 (raw file): Previously, morozovm (Maksim Morozov) wrote…
what's the resolution of this one? Comments from Reviewable |
lib/react_on_rails/utils.rb, line 77 at r2 (raw file):
Comments from Reviewable |
spec/react_on_rails/spec_helper.rb, line 30 at r1 (raw file): Previously, justin808 (Justin Gordon) wrote…
the solution is to set Test::Unit.run = true if Test::Unit defined as i did or i misunderstood the question? Comments from Reviewable |
… avoid trying to acces it in case webpacker is not used
…ebpacker manifest missing exception
56adeef
to
de1c851
Compare
changes rebased Comments from Reviewable |
Reviewed 1 of 1 files at r3. Comments from Reviewable |
This change is