Merge pull request #1752 from presidentbeef/improve_test_speed

Improve test suite speed

lib/brakeman/tracker.rb

@@ -371,7 +371,7 @@ def reset_model path
       end
     end
 
-    @models.delete model_name
+    @models.delete(model_name)
   end
 
   #Clear information related to model

test/test.rb

@@ -74,26 +74,20 @@ def warning_table type
     end
   end
 
-  def find opts = {}, &block
+  def find opts = {}
     warnings = report[warning_table(opts[:type])]
 
     opts.delete :type
 
-    result = if block
-      warnings.select block
-    else
-      warnings.select do |w|
-        opts.all? do |k,v|
-          if k == :relative_path
-            v === w.file.relative
-          else
-            v === w.send(k)
-          end
+    warnings.select do |w|
+      opts.all? do |k,v|
+        if k == :relative_path
+          v === w.file.relative
+        else
+          v === w.send(k)
         end
       end
     end
-
-    result
   end
 end
 
@@ -138,6 +132,15 @@ def test_every_warning_has_cwe_id
 module BrakemanTester::RescanTestHelper
   attr_reader :original, :rescan, :rescanner
 
+  @@temp_dirs = {}
+  @@scans = {}
+
+  Minitest.after_run do
+    @@temp_dirs.each do |_, dir|
+      FileUtils.remove_dir(dir, true)
+    end
+  end
+
   def self.included _
     unless Brakeman::Rescanner.instance_methods.include? :reindex
       Brakeman::Rescanner.class_eval do
@@ -155,26 +158,41 @@ def self.included _
   def before_rescan_of changed, app = "rails3.2", options = {}
     changed = [changed] unless changed.is_a? Array
 
-    Dir.mktmpdir do |dir|
-      @dir = dir
-      options = {:app_path => dir, :debug => false}.merge(options)
+    if @@temp_dirs[app]
+      dir = @dir = @@temp_dirs[app]
+    else
+      dir = @dir = @@temp_dirs[app] = Dir.mktmpdir('brakeman-test')
+      FileUtils.cp_r(File.join(TEST_PATH, 'apps', app, '.'), dir)
+    end
 
-      FileUtils.cp_r "#{TEST_PATH}/apps/#{app}/.", dir
-      @original = Brakeman.run options
+    options = {:app_path => dir, :debug => false}.merge(options)
 
-      yield dir if block_given?
+    if @@scans[[app, options]]
+      @original = @@scans[[app, options]]
+    else
+      @@scans[[app, options]] = @original = Brakeman.run(options)
+    end
 
-      File.open(File.join(dir, '.brakeman.dump'), "w") do |f|
-        f.print(Marshal.dump(@original))
-      end
+    begin
+      yield dir if block_given?
 
-      t = Marshal.load(File.read(File.join(dir, '.brakeman.dump')))
+      # Not reqally sure why we do this..?
+      t = Marshal.load(Marshal.dump(@original))
 
       @rescanner = Brakeman::Rescanner.new(t.options, t.processor, changed)
       @rescan = @rescanner.recheck
-
-      assert_existing
+    ensure
+      changed.each do |file|
+        original = File.join(TEST_PATH, 'apps', app, file)
+        if File.exist? original
+          FileUtils.cp original, full_path(file) 
+        else
+          FileUtils.rm full_path(file)
+        end
+      end
     end
+
+    assert_existing
   end
 
   def fixed

test/tests/parser_timeout.rb

@@ -5,14 +5,14 @@ class ParserTimeoutTests < Minitest::Test
   include BrakemanTester::RescanTestHelper
 
   def test_timeout
-    before_rescan_of "lib/large_file.rb", "rails5.2", { parser_timeout: 1 } do
+    before_rescan_of "lib/large_file.rb", "rails5.2", { parser_timeout: 0.5 } do
       random_ruby = Array.new(10000) { "def x_#{rand(1000)}\nputs '#{"**" * 1000}'\nend" }.join("\n")
       write_file "lib/large_file.rb", random_ruby
     end
 
     assert_equal 1, @rescanner.tracker.errors.length
 
     timeout_error = @rescanner.tracker.errors.first
-    assert_match(/Parsing .* took too long \(> 1 seconds\)/, timeout_error[:error])
+    assert_match(/Parsing .* took too long \(> 0.5 seconds\)/, timeout_error[:error])
   end
 end

test/tests/rescanner.rb

@@ -152,6 +152,9 @@ def test_delete_model
     model = "app/models/user.rb"
 
     before_rescan_of model do
+      # So actually there is another definition of User in
+      # app/models/user/command_dependency.rb
+      # so this does not completely delete the model
       remove model
     end
 
@@ -165,7 +168,7 @@ def test_delete_model_and_dependency
     model = "app/models/user.rb"
     dependency = "app/models/user/command_dependency.rb"
 
-    before_rescan_of model do
+    before_rescan_of [model, dependency] do
       remove model
       remove dependency
     end