prevent false 'unsatisfied' dependency status

address 2 issues causing dependency detection to falsely label an
instrumentation's dependencies as "unsatisfied".

- handle Padrino with a special case pending resolution of
  #2912
- don't mark an item as unsatisfied the second (or higher) time it comes
  around through the `detect!` loop

lib/new_relic/dependency_detection.rb

@@ -25,11 +25,9 @@ def defer(&block)
 
   def detect!
     @items.each do |item|
-      if item.dependencies_satisfied?
-        item.execute
-      else
-        item.configure_as_unsatisfied unless item.disabled_configured?
-      end
+      next if item.executed || item.disabled_configured?
+
+      item.dependencies_satisfied? ? item.execute : item.configure_as_unsatisfied
     end
   end
 
@@ -65,6 +63,13 @@ def dependencies_satisfied?
     end
 
     def configure_as_unsatisfied
+      # TODO: currently using :unsatisfied for Padrino will clobber the value
+      #       already set for Sinatra, so skip Padrino and circle back with a
+      #       new Padrino specific solution in the future.
+      #
+      #       https://github.com/newrelic/newrelic-ruby-agent/issues/2912
+      return if name == :padrino
+
       NewRelic::Agent.config.instance_variable_get(:@cache)[config_key] = :unsatisfied
     end
 

test/multiverse/suites/sinatra/sinatra_test_cases.rb

@@ -128,6 +128,13 @@ def test_with_regex_pattern
     assert_metrics_recorded(["Controller/Sinatra/#{app_name}/#{regex_segment}"])
   end
 
+  def test_that_the_actively_configured_instrumentation_is_not_marked_as_unsatsfied
+    get('/pass')
+
+    assert_equal 200, last_response.status
+    assert_includes(%w[chain prepend], NewRelic::Agent.config[:'instrumentation.sinatra'])
+  end
+
   # https://support.newrelic.com/tickets/31061
   def test_precondition_not_over_called
     get('/precondition')

test/new_relic/dependency_detection_test.rb

@@ -488,4 +488,21 @@ def test_prepend_is_replaced_by_unsatisfied_when_appropriate
       assert_equal :unsatisfied, dd.config_value
     end
   end
+
+  def test_already_executed_items_are_not_executed_again
+    unexecuted = Minitest::Mock.new
+    unexecuted.expect :executed, false
+    unexecuted.expect :dependencies_satisfied?, true
+    unexecuted.expect :disabled_configured?, false
+    unexecuted.expect :execute, -> { execution_took_place = true }
+    executed = Minitest::Mock.new
+    executed.expect :executed, true
+    unexecuted.expect :disabled_configured?, false
+
+    DependencyDetection.instance_variable_set(:@items, [unexecuted, executed])
+    DependencyDetection.detect!
+
+    unexecuted.verify
+    executed.verify
+  end
 end