mbj / mutant

Automated code reviews via mutation testing - semantic code coverage.
Other
1.95k stars 152 forks source link

Mutant creates an endless loop in Range (replaces value with Infinity) #25

Closed Stormwind closed 11 years ago

Stormwind commented 11 years ago

Hi,

I discovered another bug in mutant in combination with ranges:

@@ -3,7 +3,7 @@
     return(0)
   end
   result = 1
-  (1..exponent).each do
+  (1..((1.0) / (0.0))).each do
     result = multiply(2, result)
   end
   result
.......

It replaces the variable exponent here with (1.0) / (0.0) (Infinity), which leads to an endless loop.

Test code is still the same: https://gist.github.com/cfbba9f7f3ed0bbfa8ab

mbj commented 11 years ago

This type of mutations are valid. But I think mutant is not well prepared to handle them. I'll not emit this mutation anymore until mutant has a timeout mechanism, or blacklisting. Whatever comes first.

dkubb commented 11 years ago

@mbj I'm curious, how would you kill this kind of mutation? I was thinking about it but the solution escapes me.

mbj commented 11 years ago

@dkubb, Through the only observable property: Time. Heckle has a timer for exactly this reason. This is basically the halting problem. My plan is:

BTW I can come up with an edge case for all other mutations that would result in an endless loop.

dkubb commented 11 years ago

It would be nice if there were some rspec matchers that handle timeouts. I did a quick search for this and couldn't find anything.

Another nice to have would be a list of possible mutations and how to kill them. Things like this, observing that the code runs in less than X time, are not always obvious.

mbj commented 11 years ago

I think introducing rspec matchers (even when they are buildin) to catch these mutants would leak mutation testing to far into the examples.

As infinite mutations occur deterministically it would be more okay to disable these in the examples metadata, or preferably on the subject in config/mutant.yml or .mutant.yml. (I'll check for both in this order).

mbj commented 11 years ago

@dkubb after thinking more about this... It would be very nice to have a default timeout of 1sec for all unit examples! Forget my comment before.

around do |example|
  timeout(1) do
    example.run
  end
end

should be enough!

mbj commented 11 years ago

@stormwind You should be able to kill the mutation this way!

And sorry for posting that many comments. But my brain did not computed these thoughts in one chunk.

Stormwind commented 11 years ago

@mbj I've tried this... but it behaves a bit odd.

It's pretty fine with Rubinius:

@@ -3,7 +3,7 @@
     return(0)
   end
   result = 1
-  (1..exponent).each do
+  (1..((1.0) / (0.0))).each do
     result = multiply(2, result)
   end
   result
.......F

Failures:

  1) Calculator#two_raised_by returns 8 if your exponent is 3
     Failure/Error: subject.two_raised_by(3).should eq(8)
     Timeout::Error:
       execution expired
     # kernel/common/range.rb:69:in `each'
     [lalala more stacktrace here]
     # kernel/loader.rb:816:in `main'

Finished in 5.02 seconds
8 examples, 1 failure

Failed examples:

rspec ./spec/unit/calculator_spec.rb:51 # Calculator#two_raised_by returns 8 if your exponent is 3
Killed: rspec:Calculator#two_raised_by:/home/stormwind/beyondtdd/calculator/lib/calculator.rb:15:5bb42 (5.13s)

But it makes strange things on MRI:

@@ -3,7 +3,7 @@
     return(0)
   end
   result = 1
-  (1..exponent).each do
+  (1..((1.0) / (0.0))).each do
     result = multiply(2, result)
   end
   result
.......F

Failures:

  1) Calculator#two_raised_by returns 8 if your exponent is 3
     Failure/Error: subject.two_raised_by(3).should eq(8)
Alive: rspec:Calculator#two_raised_by:/home/stormwind/beyondtdd/calculator/lib/calculator.rb:15:5bb42 (5.37s)
@@ -3,7 +3,7 @@
     return(0)
   end
   result = 1
-  (1..exponent).each do
+  (1..((1.0) / (0.0))).each do
     result = multiply(2, result)
   end
   result

So it marks the mutant as alive here...

mbj commented 11 years ago

Can you apply the mutation by hand and rerun the specs without mutant under MRI and check the exitstatus? echo $?.

Stormwind commented 11 years ago

Hmm, it seems that MRI really crashes here:

$ rspec --color --format d

Calculator
  #add
    returns 4 if you add 2 to 2
    returns 6 if you add 3 to 3
  #multiply
    returns 4 if you multiply 2 and 2
    returns 9 if you multiply 3 and 3
  #divide
    returns 2 if you divide 4 by 2
    returns 3 if you divide 9 by 3
  #two_raised_by
    returns 0 if your exponent is 0
    returns 8 if your exponent is 3 (FAILED - 1)

Failures:

  1) Calculator#two_raised_by returns 8 if your exponent is 3
     Failure/Error: subject.two_raised_by(3).should eq(8)
/home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:203:in `dump_failure_info': undefined method `<<' for nil:NilClass (NoMethodError)
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:197:in `dump_failure'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:23:in `block in dump_failures'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:21:in `each'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:21:in `each_with_index'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/formatters/base_text_formatter.rb:21:in `dump_failures'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/reporter.rb:98:in `block in notify'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/reporter.rb:97:in `each'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/reporter.rb:97:in `notify'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/reporter.rb:81:in `finish'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/reporter.rb:36:in `report'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/command_line.rb:25:in `run'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/runner.rb:80:in `run'
    from /home/stormwind/.rvm/gems/ruby-1.9.3-p327/gems/rspec-core-2.12.2/lib/rspec/core/runner.rb:17:in `block in autorun'

Exit code is 1.

mbj commented 11 years ago

When rspec crashes mutant should also count this as mutation kill ;) Will reproduce for myself later this day.

nevir commented 11 years ago

Workaround, with the added benefit that rspec keeps going:

config.around(:each) do |spec|
  worker = Thread.new do
    spec.run
  end
  worker.join(1.0)

  if worker.alive?
    worker.kill
    raise "Spec timed out"
  end
end

(I guess timeout is using signals)

dkubb commented 11 years ago

@nevir What about the solution presented at the end of this thread: http://www.ruby-forum.com/topic/1975840

nevir commented 11 years ago

@dkubb timeout appears to throw from within a signal handler, killing the process

nevir commented 11 years ago

Or some combination of timeout + rspec + mutant is causing the process to die. If I wrap the timeout in a begin..rescue, the rescue is never entered

nevir commented 11 years ago

Ah, nope. I'm wrong from the looks of it. It's a bug in RSpec's base formatter when handling exceptions w/ a class that has no name. The stack above is what I'm getting when setting a trace, as well.

nevir commented 11 years ago

And it's been fixed on master, or whatever comes after v2.12.2

mbj commented 11 years ago

I'll close that issue now as timeouts now work to kill such mutations!

mbj commented 3 years ago

There is now also explicit support mutant side for dealing with these situations: https://github.com/mbj/mutant/blob/master/docs/configuration.md#mutation_timeout