search

testdouble / jasmine-rails

A Jasmine runner for rails projects that's got you covered in both the terminal and the browser
http://rubygems.org/gems/jasmine-rails
MIT License
378 stars 154 forks source link

JRuby issue fix for bundle exec spec:javascript #176

Closed pironim closed 8 years ago

pironim commented 8 years ago

fix weird behaviour for jruby - output disappear from screen appear on jruby 1.7.16.1 only when you type bundle exec spec:javascript without RAILS_ENV=test in the begining. with RAILS_ENV=test it working..

Resolves #177

searls commented 8 years ago

Before merging I really need someone to make sure that exit codes are still working properly in MRI

pironim commented 8 years ago

@searls I can check other ruby versions (thanks to rbenv).. how many do you need and is there any edge cases to test?

searls commented 8 years ago

Just make sure it works in MRI 2.2 On Sun, Nov 8, 2015 at 14:21 Vladimir Zhukov notifications@github.com wrote:

@searls https://github.com/searls I can check other ruby versions (thanks to rbenv).. how many do you need and is there any edge cases to test?

— Reply to this email directly or view it on GitHub https://github.com/searls/jasmine-rails/pull/176#issuecomment-154864517.

pironim commented 8 years ago

@searls Here is my tests.. I change permissions for spec folder and run rake task http://screencloud.net/v/n5cC than I retur parmissions back and run task https://screencloud.net/v/yfjn

Let me know it's enough or not ..

searls commented 8 years ago

Hey @pironim, those both link to the same screenshot. I wasn't aware—what does this issue have to do with permissions?

pironim commented 8 years ago

@searls I test exit code when modifying permissions. I update my prev comment and paste correct link

searls commented 8 years ago

Tested locally. Looks good. Thanks!

searls commented 8 years ago

landed in 0.12.2

pironim commented 8 years ago

Thanks a lot

pironim commented 8 years ago

I know it already merged but exit codes was before for some reason . @bunnymatic add them. I just want to notify author about those changes.. Can you look what is done here and check if it's possible.. Just double check to make sure nothing is broken.

searls commented 8 years ago

What? I don't understand your message. On Thu, Nov 12, 2015 at 06:21 Vladimir Zhukov notifications@github.com wrote:

I know it already merged but exit codes was before for some reason . @bunnymatic https://github.com/bunnymatic add them. I just want to notify author about those changes.. Can you look what is done here and check if it's possible.. Just double check to make sure nothing is broken.

— Reply to this email directly or view it on GitHub https://github.com/searls/jasmine-rails/pull/176#issuecomment-156077982.

pironim commented 8 years ago

@searls I'm asking another contributor to check (if it's possible) what is done here. @bunnymatic Can you look at my changes here if you have a time.

bunnymatic commented 8 years ago

@pironim Things look fine to me. And I suspect the exit code is the same as it was when we used system instead of exec.

exec should "replaces the current process by running the given external command" (from http://ruby-doc.org/core-2.2.3/Kernel.html#method-i-exec) where as system "executes a command in a subshell".

I don't know what issues you're seeing now, but I would assume exec should work fine here and things should behave the same. I tested locally and (with just a spot check) it seems like things are running properly

# with a green test suite
rake spec:javascript && echo "STATUS" $?
Running `"phantomjs" "" "/Users/jon/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/jasmine-rails-0.12.2/lib/jasmine_rails/../assets/javascripts/jasmine-runner.js" "file:///Users/jon/projects/myproject/tmp/jasmine/runner.html?spec="`
Running: file:///Users/jon/projects/myproject/tmp/jasmine/runner.html?spec=
Starting...

Finished
-----------------
39 specs, 0 failures in 0.06s.

ConsoleReporter finished

STATUS 0

and a failing suite

rake spec:javascript; echo "STATUS" $?
Running `"phantomjs" "" "/Users/jon/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/jasmine-rails-0.12.2/lib/jasmine_rails/../assets/javascripts/jasmine-runner.js" "file:///Users/jon/projects/myproject/tmp/jasmine/runner.html?spec="`
Running: file:///Users/jon/projects/myproject/tmp/jasmine/runner.html?spec=
Starting...

<snip> error output </snip>

Finished
-----------------
39 specs, 1 failure in 0.08s.

ConsoleReporter finished

rake aborted!
Error executing command: "phantomjs" "" "/Users/jon/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/jasmine-rails-0.12.2/lib/jasmine_rails/../assets/javascripts/jasmine-runner.js" "file:///Users/jon/projects/myproject/tmp/jasmine/runner.html?spec="

Tasks: TOP => spec:javascript
(See full trace by running task with --trace)
STATUS 1

What is the problem you're seeing?

pironim commented 8 years ago

@bunnymatic I don't have a problem with current solution.. just blame myself to ship code without good tests ... and having some fear about results..

bunnymatic commented 8 years ago

From what i can see, i think it's good. I don't use jRuby but am mostly on the current MRI build so if it works for you and it works for me, i think we're probably good!

On Thu, Nov 12, 2015 at 8:17 AM, Vladimir Zhukov notifications@github.com wrote:

@bunnymatic https://github.com/bunnymatic I don't have a problem with current solution.. just blame myself to ship code without good tests ... and having some fear about results..

— Reply to this email directly or view it on GitHub https://github.com/searls/jasmine-rails/pull/176#issuecomment-156152835.

searls commented 8 years ago

I tested it @pironim, I think we're ok. This is a really hard gem to write that sort of test for.