Fix rails console --sandbox test for Mac OS

[MacksMind]

Using Process#kill was triggering ReportCrash on Mac OS Moutain Lion and
the test would not complete. This happened whether run from rake test
or as ruby -w -Itest ...

/cc @jonleighton @carlosantoniodasilva

[carlosantoniodasilva]

Hm yeah, I was having problems but couldn't look yet. Will try your patch later, thanks @MacksMind!

[carlosantoniodasilva]

Can you move this default to the arguments definition instead?

[carlosantoniodasilva]

Confirme, tests pass for me with this change.

@jonleighton thoughts?

[rrmartins]

@carlosantoniodasilva not passed the tests...

[carlosantoniodasilva]

@rrmartins sorry, what? It seems to be passing here.

[rrmartins]

@carlosantoniodasilva here in my machine had a failure, below is the log:

1) Failure:
FullStackConsoleTest#test_sandbox [test/application/console_test.rb:125]:
"Post.count" expected, but got:

quit

[carlosantoniodasilva]

@rrmartins crap.. which version is your OS?

[rrmartins]

@carlosantoniodasilva here is my version 10.8.3

[MacksMind]

@rrmartins Pulling that down now.

[carlosantoniodasilva]

Weird, I'm already on OSX 10.8.3, and it worked.

[MacksMind]

Works for me on 10.8.3 too. @rrmartins, call you try increasing the timeout at line 109 and see if anything changes. Grasping at straws, but it seems like that quit should have already been consumed.

[jonleighton]

Sorry for my slow reply folks, I've been moving house :( :( :( I'll look at this today.

[jonleighton]

Hi guys, I hope that the above commit resolves this? Please let me know if not.

[MacksMind]

No, it doesn't.

$ bundle exec ruby -wItest test/application/console_test.rb
Run options: --seed 41388

# Running tests:

.....Terminated: 15

[MacksMind]

Based on the Process#kill rdoc, what you did looks good. But I played with it for a while on my Mac and I couldn't figure out how to make it do what I wanted. My patch isn't ideal, but it's the best I could think of. Do we need to bring this up at http://bugs.ruby-lang.org/ ?

[jonleighton]

Hmm, weird. Maybe some native extension is objecting to the signal. Anyway, how about this?

diff --git a/railties/test/application/console_test.rb b/railties/test/application/console_test.rb
index d586822..80700a1 100644
--- a/railties/test/application/console_test.rb
+++ b/railties/test/application/console_test.rb
@@ -126,12 +126,6 @@ class FullStackConsoleTest < ActiveSupport::TestCase
     assert_output "> "
   end

-  def kill(pid)
-    Process.kill('TERM', pid)
-    Process.wait(pid)
-  rescue Errno::ESRCH
-  end
-
   def spawn_console
     pid = Process.spawn(
       "#{app_path}/bin/rails console --sandbox",
@@ -148,15 +142,13 @@ class FullStackConsoleTest < ActiveSupport::TestCase
     write_prompt "Post.count", "=> 0"
     write_prompt "Post.create"
     write_prompt "Post.count", "=> 1"
-
-    kill pid
+    @master.puts "quit"

     pid = spawn_console

     write_prompt "Post.count", "=> 0"
     write_prompt "Post.transaction { Post.create; raise }"
     write_prompt "Post.count", "=> 0"
-  ensure
-    kill pid if pid
+    @master.puts "quit"
   end
 end

I basically want to avoid having special cases for different OSes.

[MacksMind]

Yes!

[jonleighton]

Ok, I've committed that then

[MacksMind]

@jonleighton Thanks