Process spawn should not raise NoMethodError when true argument is given to :pgroup#1895
Process spawn should not raise NoMethodError when true argument is given to :pgroup#1895dbussink merged 2 commits intorubinius:masterfrom LTe:process_spawn
Conversation
There was a problem hiding this comment.
We should try to spec behavior here as how it should work, not that it shouldn't raise an exception. So that means we should specify here that it creates a new pgroup.
In general, any spec that says "should not raise an exception" is almost always a wrong spec.
There was a problem hiding this comment.
I update pull request. And spec about "should not raise and exception" can be good: https://github.com/LTe/rubinius/commit/a16eeac790527589e862b03d0aab95ae927137b3#L0R272 :)
|
|
|
Pass on travis but On my localhost: |
There was a problem hiding this comment.
Looks like this description is wrong. It raises an exception and the description says otherwise.
We need to consider several options
I moved a little bit of logic from this place to this place Now when @dbussink what do you think? |
There was a problem hiding this comment.
Looks like we have multiple if value checks here. What is the behavior when value is nil? We should try to only have to this check once probably.
There was a problem hiding this comment.
When pgroup is nil or false process group will not create.
There was a problem hiding this comment.
@dbussink we can do something like that:
value = 0 if value == true
if value
value = Rubinius::Type.coerce_to value, Integer, :to_int
raise ArgumentError, "negative process group ID : #{value}" if value < 0
endThere was a problem hiding this comment.
How about something like this?
if value == true
value = 0
elsif value
value = Rubinius::Type.coerce_to value, Integer, :to_int
raise ArgumentError, "negative process group ID : #{value}" if value < 0
endWhen user execute method with :pgroup => true Rubinius tried to use '<' method on TrueClass object. It will throw an exception NoMethotError. Process.spawn executed with :pgroup => Symbol should raise TypeError
Cases: 1. pgroup value is 'true' => 0 2. pgroup value is kind of true => Rubinius will try coerce_to Integer 3. pgroup value if kind of true and pass coerce then Rubinius will check check whether the value is < 0. If yes then an ArgumentError exception is thrown 4. pgroup if kind of false (false, nil) - default behavior (like without pgroup) Fixes #1894
Process spawn should not raise NoMethodError when true argument is given to :pgroup
No description provided.