Patches: Browse | Submit New | Admin

[#22829] Fixes endless loop bug and cleans up code

Date:
2008-11-15 11:30
Priority:
3
Submitted By:
Petrik de Heus (peet4130)
Assigned To:
Kevin Clark (kevinclark)
Category:
heckle
State:
Open
Summary:
Fixes endless loop bug and cleans up code

Detailed description
Fixes:

- The endless loop bug:

    if stack.first != "process_iter" then
      mutate_node out
    else
+      @mutatees.delete out # this line was missing and caused an endless loop
      out
    end

- Fixes test for ParseTree 3.0.2
- Cleans up the method aliasing (no more rescue nil and forgotten aliases)
- Seperates some concerns by moving mutatees logic to a seperate class


diff --git a/lib/heckle.rb b/lib/heckle.rb
index 7ba27c4..b9c6f73 100644
--- a/lib/heckle.rb
+++ b/lib/heckle.rb
@@ -46,11 +46,6 @@ class Heckle < SexpProcessor
   attr_accessor :count
 
   ##
-  # Mutations that caused failures
-
-  attr_accessor :failures
-
-  ##
   # Class being heckled
 
   attr_accessor :klass
@@ -71,8 +66,6 @@ class Heckle < SexpProcessor
   attr_accessor :method_name
 
   attr_accessor :mutatees # :nodoc:
-  attr_accessor :mutation_count # :nodoc:
-  attr_accessor :node_count # :nodoc:
   attr_accessor :original_tree # :nodoc:
 
   @@debug = false
@@ -112,22 +105,10 @@ class Heckle < SexpProcessor
     self.auto_shift_type = true
     self.expected = Array
 
-    @mutatees = Hash.new
-    @mutation_count = Hash.new
-    @node_count = Hash.new
+    @mutatees = Mutatees.new nodes, current_tree
     @count = 0
 
-    @mutatable_nodes = nodes
-    @mutatable_nodes.each {|type| @mutatees[type] = [] }
-
-    @failures = []
-
-    @mutated = false
-
-    grab_mutatees
-
     @original_tree = current_tree.deep_clone
-    @original_mutatees = mutatees.deep_clone
   end
 
   ##
@@ -139,7 +120,7 @@ class Heckle < SexpProcessor
 
   def run_tests
     if tests_pass? then
-      record_passing_mutation
+      @reporter.add_passing_mutation(current_code)
     else
       @reporter.report_test_failures
     end
@@ -166,50 +147,31 @@ class Heckle < SexpProcessor
         @reporter.warning "Mutation caused a syntax error:\n\n#{e.message}}"
       rescue Timeout::Error
         @reporter.warning "Your tests timed out. Heckle may have caused an infinite loop."
-      end
-    end
-
-    reset # in case we're validating again. we should clean up.
-
-    unless @failures.empty?
-      @reporter.no_failures
-      @failures.each do |failure|
-        original = RubyToRuby.new.process(@original_tree.deep_clone)
-        @reporter.failure(original, failure)
-      end
-      false
-    else
-      @reporter.no_surviving_mutants
-      true
+      end      
     end
-  end
 
-  def record_passing_mutation
-    @failures << current_code
+    # in case we're validating again. we should clean up.
+    reset
+    restore_method
+    @reporter.display_failures(@original_tree.deep_clone)
   end
 
   def heckle(exp)
     exp_copy = exp.deep_clone
     src = begin
+            #TODO add timeout block. The following exp takes forever
+            # [:defn, :id, [:fbody, [:scope, [:block, [:args], [:and, [:lasgn, :v, [:nil]],
+            #  [:rescue, [:call, [:lvar, :v], :to_i], [:resbody, nil, [:if, [:lvar, :v], [:lit, 1], [:lit, 0]]]]]]]]])
             RubyToRuby.new.process(exp)
           rescue => e
             puts "Error: #{e.message} with: #{klass_name}##{method_name}: #{exp_copy.inspect}"
             raise e
           end
-
-    original = RubyToRuby.new.process(@original_tree.deep_clone)
-    @reporter.replacing(klass_name, method_name, original, src) if @@debug
-
-    clean_name = method_name.to_s.gsub(/self\./, '')
-    self.count += 1
-    new_name = "h#{count}_#{clean_name}"
-
-    klass = aliasing_class method_name
-    klass.send :remove_method, new_name rescue nil
-    klass.send :alias_method, new_name, clean_name
-    klass.send :remove_method, clean_name rescue nil
-
-    @klass.class_eval src, "(#{new_name})"
+    @reporter.replacing(klass_name, method_name, @original_tree, src) if @@debug
+    
+    restore_method
+    alias_new_method    
+    @klass.class_eval src, "(#{new_name(method_name)})"
   end
 
   ############################################################
@@ -228,6 +190,7 @@ class Heckle < SexpProcessor
     if stack.first != "process_iter" then
       mutate_node out
     else
+      @mutatees.delete out
       out
     end
   end
@@ -244,11 +207,11 @@ class Heckle < SexpProcessor
     result = [:defn, method]
     result << process(exp.shift) until exp.empty?
     heckle(result) if method == method_name
-
+    
     return result
   ensure
     @mutated = false
-    reset_node_count
+    @mutatees.reset_node_count
   end
 
   ##
@@ -431,10 +394,9 @@ class Heckle < SexpProcessor
 
   def mutate_node(node)
     raise UnsupportedNodeError unless respond_to? "mutate_#{node.first}"
-    increment_node_count node
-
+    @mutatees.increment_node_count(node)
     if should_heckle? node then
-      increment_mutation_count node
+      @mutatees.delete(node)
       return send("mutate_#{node.first}", node)
     else
       node
@@ -444,71 +406,17 @@ class Heckle < SexpProcessor
   ############################################################
   ### Tree operations
 
-  def walk_and_push(node)
-    return unless node.respond_to? :each
-    return if node.is_a? String
-    node.each { |child| walk_and_push(child) }
-    if @mutatable_nodes.include? node.first
-      @mutatees[node.first.to_sym].push(node)
-      mutation_count[node] = 0
-    end
-  end
-
-  def grab_mutatees
-    walk_and_push(current_tree)
-  end
-
   def current_tree
     ParseTree.translate(klass_name.to_class, method_name)
   end
 
   def reset
     reset_tree
-    reset_mutatees
-    reset_mutation_count
+    @mutatees.reset
   end
 
   def reset_tree
-    return unless original_tree != current_tree
-    @mutated = false
-
-    self.count += 1
-
-    clean_name = method_name.to_s.gsub(/self\./, '')
-    new_name = "h#{count}_#{clean_name}"
-
-    klass = aliasing_class method_name
-
-    klass.send :undef_method, new_name rescue nil
-    klass.send :alias_method, new_name, clean_name
-    klass.send :alias_method, clean_name, "h1_#{clean_name}"
-  end
-
-  def reset_mutatees
-    @mutatees = @original_mutatees.deep_clone
-  end
-
-  def reset_mutation_count
-    mutation_count.each {|k,v| mutation_count[k] = 0}
-  end
-
-  def reset_node_count
-    node_count.each {|k,v| node_count[k] = 0}
-  end
-
-  def increment_node_count(node)
-    if node_count[node].nil?
-      node_count[node] = 1
-    else
-      node_count[node] += 1
-    end
-  end
-
-  def increment_mutation_count(node)
-    # So we don't re-mutate this later if the tree is reset
-    mutation_count[node] += 1
-    @mutatees[node.first].delete_at(@mutatees[node.first].index(node))
-    @mutated = true
+    restore_method unless original_tree == current_tree
   end
 
   ############################################################
@@ -517,12 +425,24 @@ class Heckle < SexpProcessor
   def aliasing_class(method_name)
     method_name.to_s =~ /self\./ ? class << @klass; self; end : @klass
   end
+  
+  def restore_method
+    klass = aliasing_class method_name        
+    if klass.send  :method_defined?, new_name(method_name)
+      klass.send :alias_method, clean_name(method_name), new_name(method_name)    
+      klass.send :remove_method, new_name(method_name)
+    end
+  end 
+
+  def alias_new_method
+    klass = aliasing_class method_name    
+    self.count += 1
+    klass.send :alias_method, new_name(method_name), clean_name(method_name)    
+  end
 
   def should_heckle?(exp)
     return false unless method == method_name
-    mutation_count[exp] = 0 if mutation_count[exp].nil?
-    return false if node_count[exp] <= mutation_count[exp]
-    ( mutatees[exp.first.to_sym] || [] ).include?(exp) && !already_mutated?
+    return @mutatees.not_mutated?(exp)
   end
 
   def grab_conditional_loop_parts(exp)
@@ -532,20 +452,23 @@ class Heckle < SexpProcessor
     return cond, body, head_controlled
   end
 
-  def already_mutated?
-    @mutated
-  end
-
   def mutations_left
-    sum = 0
-    @mutatees.each {|mut| sum += mut.last.size }
-    sum
+    @mutatees.mutations_left
   end
 
   def current_code
     RubyToRuby.translate(klass_name.to_class, method_name)
   end
 
+  def new_name(method_name)
+    clean_name = clean_name(method_name)
+    "h#{count}_#{clean_name}"
+  end
+  
+  def clean_name(method_name)
+    method_name.to_s.gsub(/self\./, '')    
+  end
+
   ##
   # Returns a random Fixnum.
 
@@ -609,6 +532,11 @@ class Heckle < SexpProcessor
   end
 
   class Reporter
+
+    def initialize
+      @failures = []
+    end
+
     def no_mutations(method_name)
       warning "#{method_name} has a thick skin. There's nothing to heckle."
     end
@@ -661,22 +589,113 @@ class Heckle < SexpProcessor
       end
     end
 
-    def failure(original, failure)
-      self.diff original, failure
-    end
-
     def no_surviving_mutants
       puts "No mutants survived. Cool!\n\n"
     end
 
-    def replacing(klass_name, method_name, original, src)
+    def replacing(klass_name, method_name, original_tree, src)
+      original = RubyToRuby.new.process(original_tree.deep_clone)
       puts "Replacing #{klass_name}##{method_name} with:\n\n"
       diff(original, src)
     end
 
+    def display_failures(original_tree)
+      unless @failures.empty?
+        no_failures
+        @failures.each do |failure|
+          original = RubyToRuby.new.process(original_tree)
+          diff(original, failure)
+        end
+        false
+      else
+        no_surviving_mutants
+        true
+      end
+    end
+
     def report_test_failures
       puts "Tests failed -- this is good"
     end
+
+    def add_passing_mutation(code)
+      @failures << code      
+    end
+  end
+
+  class Mutatees
+    
+    attr_accessor :mutatees    
+    
+    def initialize(nodes, current_tree)
+      @mutatees = Hash.new
+      @mutation_count = Hash.new
+      @node_count = Hash.new
+            
+      @mutatable_nodes = nodes
+      @mutatable_nodes.each {|type| @mutatees[type] = [] }
+      walk_and_push(current_tree)
+      @original_mutatees = @mutatees.deep_clone
+      
+    end      
+    
+    def mutations_left
+      sum = 0
+      @mutatees.each {|mut| sum += mut.last.size }
+      sum
+    end    
+    
+    def reset
+      @mutated = false
+      @mutatees = @original_mutatees.deep_clone
+      @mutation_count.each {|k,v| @mutation_count[k] = 0}
+    end
+    
+    def reset_node_count
+      @mutated = false      
+      @node_count.each {|k,v| @node_count[k] = 0}
+    end
+    
+    def increment_node_count(node)
+      @node_count[node] = 0 if @node_count[node].nil?
+      @node_count[node] += 1
+    end
+    
+    def not_mutated?(exp)
+      return false if @mutated
+      return false if all_nodes_done?(exp)
+      ( @mutatees[exp.first.to_sym] || [] ).include?(exp)      
+    end
+
+    def delete(exp)
+      index = @mutatees[exp.first.to_sym].index(exp)
+      if index
+        @mutatees[exp.first.to_sym].delete_at(index)
+        @mutation_count[exp] += 1
+        @mutated = true
+      end
+    end    
+    
+    private
+    
+    def walk_and_push(node)
+      return unless node.respond_to? :each
+      return if node.is_a? String
+      node.each { |child| walk_and_push(child) }
+      push(node)
+    end
+        
+    def push(node)
+      if node.first && @mutatable_nodes.include?(node.first.to_sym)
+        @mutatees[node.first.to_sym].push(node)
+        @mutation_count[node] = 0
+      end
+    end
+    
+    def all_nodes_done?(exp)
+      mutation_count = @mutation_count[exp] || 0
+      node_count = @node_count[exp] || 0      
+      node_count <= mutation_count
+    end    
   end
 
   ##
diff --git a/test/test_heckle.rb b/test/test_heckle.rb
index 67ad818..2fc6aa6 100644
--- a/test/test_heckle.rb
+++ b/test/test_heckle.rb
@@ -59,12 +59,10 @@ class LiteralHeckleTestCase < HeckleTestCase
     assert_equal util_expected(1), @heckler.current_tree
 
     @heckler.reset_tree
-
     @heckler.process(@heckler.current_tree)
     assert_equal util_expected(2), @heckler.current_tree
 
     @heckler.reset_tree
-
     @heckler.process(@heckler.current_tree)
     assert_equal util_expected(3), @heckler.current_tree
   end
@@ -134,7 +132,7 @@ class TestHeckle < HeckleTestCase
       :until => [[:until, [:vcall, :some_func], [:vcall, :some_other_func], true]]
     }
 
-    assert_equal expected, @heckler.mutatees
+    assert_equal expected, @heckler.mutatees.mutatees
   end
 
   def test_should_count_mutatees_left
@@ -143,16 +141,16 @@ class TestHeckle < HeckleTestCase
 
   def test_reset
     original_tree = @heckler.current_tree.deep_clone
-    original_mutatees = @heckler.mutatees.deep_clone
+    original_mutatees = @heckler.mutatees.mutatees.deep_clone
 
     3.times { @heckler.process(@heckler.current_tree) }
 
     assert_not_equal original_tree, @heckler.current_tree
-    assert_not_equal original_mutatees, @heckler.mutatees
+    assert_not_equal original_mutatees, @heckler.mutatees.mutatees
 
     @heckler.reset
     assert_equal original_tree[2], @heckler.current_tree[2][1]
-    assert_equal original_mutatees, @heckler.mutatees
+    assert_equal original_mutatees, @heckler.mutatees.mutatees
   end
 
   def test_reset_tree
@@ -167,34 +165,76 @@ class TestHeckle < HeckleTestCase
 
   def test_reset_should_work_over_several_process_calls
     original_tree = @heckler.current_tree.deep_clone
-    original_mutatees = @heckler.mutatees.deep_clone
+    original_mutatees = @heckler.mutatees.mutatees.deep_clone
 
     @heckler.process(@heckler.current_tree)
     assert_not_equal original_tree, @heckler.current_tree
-    assert_not_equal original_mutatees, @heckler.mutatees
+    assert_not_equal original_mutatees, @heckler.mutatees.mutatees
 
     @heckler.reset
     assert_equal original_tree, @heckler.current_tree
-    assert_equal original_mutatees, @heckler.mutatees
+    assert_equal original_mutatees, @heckler.mutatees.mutatees
 
     3.times { @heckler.process(@heckler.current_tree) }
     assert_not_equal original_tree, @heckler.current_tree
-    assert_not_equal original_mutatees, @heckler.mutatees
+    assert_not_equal original_mutatees, @heckler.mutatees.mutatees
 
     @heckler.reset
     assert_equal original_tree, @heckler.current_tree
-    assert_equal original_mutatees, @heckler.mutatees
+    assert_equal original_mutatees, @heckler.mutatees.mutatees
   end
 
   def test_reset_mutatees
-    original_mutatees = @heckler.mutatees.deep_clone
+    original_mutatees = @heckler.mutatees.mutatees.deep_clone
 
     @heckler.process(@heckler.current_tree)
-    assert_not_equal original_mutatees, @heckler.mutatees
+    assert_not_equal original_mutatees, @heckler.mutatees.mutatees
 
-    @heckler.reset_mutatees
-    assert_equal original_mutatees, @heckler.mutatees
+    @heckler.reset
+    assert_equal original_mutatees, @heckler.mutatees.mutatees
   end
+  
+  def test_restore_method
+    @heckler.process(@heckler.current_tree)
+    assert_equal ["h1_uses_many_things", "hash"], @heckler.klass.new.methods.grep(/^h/).sort    
+    @heckler.process(@heckler.current_tree)
+    assert_equal ["h2_uses_many_things", "hash"], @heckler.klass.new.methods.grep(/^h/).sort    
+    @heckler.reset
+    assert_equal ["hash"], @heckler.klass.new.methods.grep(/^h/).sort
+  end  
+end
+
+class HeckleConvenienceMethodsTestCase < Test::Unit::TestCase
+  
+  def setup
+    @heckler = TestHeckler.new("Heckled")
+  end
+
+  def test_tests_pass_should_throw_not_implmented
+    assert_raises(NotImplementedError){Heckle.new("Heckled").tests_pass?}
+  end
+
+  def test_new_name
+    assert_equal("h0_rand_string", @heckler.new_name(:rand_string))
+  end
+
+  def test_clean_name
+    assert_equal("abc", @heckler.clean_name('self.abc'))
+  end
+  
+  def test_aliasing_class    
+    assert_equal(Heckled, @heckler.aliasing_class('abc'))
+  end
+  
+  def test_aliasing_class_meta_class
+    meta = class << Heckled; self; end
+    assert_equal(meta, @heckler.aliasing_class('self.abc'))
+  end  
+#   def test_set_timeout
+#     Heckle.timeout = 3
+#     assert_equal 4, Heckle.new.timeout
+# TestHeckler.new("Heckled", "uses#{data}", @nodes)    
+#   end  
 end
 
 class TestHeckleNumericLiterals < HeckleTestCase
@@ -455,6 +495,18 @@ class TestHeckleCallblock < HeckleTestCase
     assert_equal expected, @heckler.current_tree
   end
 
+  def test_callblock_deleted_also_removes_it_from_mutatees
+    expected = [:defn, :uses_callblock,
+                [:scope,
+                 [:block,
+                  [:args],
+                  [:iter, [:call, [:vcall, :x], :y], nil, [:lit, 1]]]]]
+
+    assert_equal({:call=>[[:call, [:vcall, :x], :y]]}, @heckler.mutatees.mutatees)
+    @heckler.process(@heckler.current_tree)
+    assert_equal({:call=>[]}, @heckler.mutatees.mutatees)
+  end
+
 end
 
 class TestHeckleClassMethod < Test::Unit::TestCase
@@ -467,7 +519,7 @@ class TestHeckleClassMethod < Test::Unit::TestCase
   end
   
   def test_default_structure
-    expected = [:defn, :"self.is_a_klass_method?",
+    expected = [:defs, [:self], :is_a_klass_method?,
                 [:scope,
                  [:block,
                   [:args],
@@ -476,11 +528,11 @@ class TestHeckleClassMethod < Test::Unit::TestCase
   end
   
   def test_heckle_class_methods
-    expected = [:defn, :"self.is_a_klass_method?",
+    expected = [:defs, [:self], :is_a_klass_method?,
                 [:scope,
                  [:block,
                   [:args],
-                  [:false]]]]
+                  [:true]]]]
     @heckler.process(@heckler.current_tree)
     assert_equal expected, @heckler.current_tree
   end
@@ -739,11 +791,11 @@ class TestHeckleMasgn < HeckleTestCase
                       [:lasgn, :_heckle_dummy],
                       [:gasgn, :$b],
                       [:lasgn, :c]],
+                    nil,
                     [:array, [:lit, 5], [:lit, 6], [:lit, 7]]]]]]
 
     @heckler.process(@heckler.current_tree)
     assert_equal expected, @heckler.current_tree
   end
 
-end
-
+end
\ No newline at end of file

Add A Comment: Notepad

Please login


Followup

Message
Date: 2008-11-15 12:20
Sender: Petrik de Heus

Ok this works with:
ParseTree-3.0.2
ruby2ruby-1.1.8
ruby 1.8.6 (2008-08-11 patchlevel 287) [i686-darwin9.5.0]
I'll try to patch it for ruby2ruby-1.2.1 as well.

Attached Files:

Name Description Download
No Files Currently Attached

Changes:

No Changes Have Been Made to This Item