Allow skipping/failing resources in FilterTable (#2349)

* Allow skipping/failing resources in FilterTable

`FilterTable` is commonly used in the class body of a resource and is
evaluated during an `instance_eval`. This means that if you raise an
exception (e.g. SkipResource) it will halt `inspec exec` and
`inspec check`.

This adds an `ExceptionCatcher` class that will postpone evaluation
until test execution.

This allows `inspec check` and `inspec exec` to perform as intended when
skipping/failing a resource in `FilterTable`

Huge thanks to @adamleff for providing the starting code/ideas!

Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>

* Comment why `ExceptionCatcher` doesn't raise

Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>

* Remove `accessor` from `ExceptionCatcher`

Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>

* Return the existing ExceptionCatcher object rather than creating new

Signed-off-by: Adam Leff <adam@leff.co>

lib/utils/filter.rb

@@ -6,6 +6,48 @@
 module FilterTable
   module Show; end
 
+  class ExceptionCatcher
+    def initialize(original_resource, original_exception)
+      @original_resource = original_resource
+      @original_exception = original_exception
+    end
+
+    # This method is called via the runner and signals RSpec to output a block
+    # showing why the resource was skipped. This prevents the resource from
+    # being added to the test collection and being evaluated.
+    def resource_skipped?
+      @original_exception.is_a?(Inspec::Exceptions::ResourceSkipped)
+    end
+
+    # This method is called via the runner and signals RSpec to output a block
+    # showing why the resource failed. This prevents the resource from
+    # being added to the test collection and being evaluated.
+    def resource_failed?
+      @original_exception.is_a?(Inspec::Exceptions::ResourceFailed)
+    end
+
+    def resource_exception_message
+      @original_exception.message
+    end
+
+    # Capture message chains and return `ExceptionCatcher` objects
+    def method_missing(*)
+      self
+    end
+
+    # RSpec will check the object returned to see if it responds to a method
+    # before calling it. We need to fake it out and tell it that it does. This
+    # allows it to skip past that check and fall through to #method_missing
+    def respond_to?(_method)
+      true
+    end
+
+    def to_s
+      @original_resource.to_s
+    end
+    alias inspect to_s
+  end
+
   class Trace
     def initialize
       @chain = []
@@ -140,7 +182,7 @@ module FilterTable
       @connectors = {}
     end
 
-    def connect(resource, table_accessor)
+    def connect(resource, table_accessor) # rubocop:disable Metrics/AbcSize
       # create the table structure
       connectors = @connectors
       struct_fields = connectors.values.map(&:field_name)
@@ -170,12 +212,21 @@ module FilterTable
         end
       }
 
-      # define all access methods with the parent resource
+      # Define all access methods with the parent resource
+      # These methods will be configured to return an `ExceptionCatcher` object
+      # that will always return the original exception, but only when called
+      # upon. This will allow method chains in `describe` statements to pass the
+      # `instance_eval` when loaded and only throw-and-catch the exception when
+      # the tests are run.
       accessors = @accessors + @connectors.keys
       accessors.each do |method_name|
         resource.send(:define_method, method_name.to_sym) do |*args, &block|
-          filter = table.new(self, method(table_accessor).call, ' with')
-          filter.method(method_name.to_sym).call(*args, &block)
+          begin
+            filter = table.new(self, method(table_accessor).call, ' with')
+            filter.method(method_name.to_sym).call(*args, &block)
+          rescue Inspec::Exceptions::ResourceFailed, Inspec::Exceptions::ResourceSkipped => e
+            FilterTable::ExceptionCatcher.new(resource, e)
+          end
         end
       end
     end

test/functional/inspec_check_test.rb

@@ -22,4 +22,11 @@ describe 'inspec check' do
       out.exit_status.must_equal 0
     end
   end
+
+  describe 'inspec check with skipping/failing a resource in FilterTable' do
+    it 'can check a profile with special characters in its path' do
+      out = inspec('check ' + File.join(profile_path, 'profile-with-resource-exceptions'))
+      out.exit_status.must_equal 0
+    end
+  end
 end

test/unit/mock/profiles/profile-with-resource-exceptions/controls/exception_resource.rb

@@ -1,31 +1,74 @@
 # encoding: utf-8
 
+# checks[0]
 describe exception_resource_test('should raise ResourceSkipped', :skip_me) do
   its('value') { should eq 'does not matter' }
 end
 
+# checks[1]
 describe exception_resource_test('should raise ResourceFailed', :fail_me) do
   its('value') { should eq 'does not matter' }
 end
 
+# checks[2]
 describe exception_resource_test('should pass') do
   its('value') { should eq 'should pass' }
 end
 
+# checks[3]
 describe exception_resource_test('fail inside matcher') do
   its('inside_matcher') { should eq 'does not matter' }
 end
 
+# checks[4]
 describe exception_resource_test('skip inside matcher') do
   its('inside_matcher') { should eq 'does not matter' }
 end
 
 control 'should-work-within-control' do
+  # checks[5][0]
   describe exception_resource_test('should skip', :skip_me) do
     its('value') { should eq 'does not matter' }
   end
 
+  # checks[5][1]
   describe exception_resource_test('should fail', :fail_me) do
     its('value') { should eq 'does not matter' }
   end
 end
+
+# checks[6]
+describe exception_resource_test('skip_me').matters('does not matter') do
+  its('matters') { should eq 'does not matter' }
+end
+
+# checks[7]
+describe exception_resource_test('fail_me').matters('does not matter') do
+  its('matters') { should eq 'does not matter' }
+end
+
+# checks[8]
+describe exception_resource_test('skip_me').matters('it really does').another_filter('example') do
+  its('value') { should cmp 'does not matter' }
+end
+
+# checks[9]
+describe exception_resource_test('fail_me').matters('it really does').another_filter('example') do
+  its('value') { should cmp 'does not matter' }
+end
+
+# checks[10]
+describe exception_resource_test('skip_me').matters('it really does').not_real_filter('example') do
+  its('value') { should cmp 'does not matter' }
+end
+
+# checks[11]
+describe exception_resource_test('fail_me').matters('it really does').not_real_filter('example') do
+  its('value') { should cmp 'does not matter' }
+end
+
+# checks[12]
+describe exception_resource_test('should_pass').matters('it really does') do
+  its('another_filter') { should cmp 'example' }
+end
+

test/unit/mock/profiles/profile-with-resource-exceptions/libraries/example_resource.rb

@@ -44,6 +44,25 @@ class ExceptionResourceTest < Inspec.resource(1)
     end
   end
 
+  filter = FilterTable.create
+  filter.add_accessor(:where)
+        .add_accessor(:entries)
+        .add(:matters,  field: 'matters')
+        .add(:another_filter,  field: 'another_filter')
+        .connect(self, :filters_example)
+
+  private
+
+  def filters_example
+    case @value
+    when 'skip_me'
+      raise Inspec::Exceptions::ResourceSkipped, 'Skipping inside FilterTable'
+    when 'fail_me'
+      raise Inspec::Exceptions::ResourceFailed, 'Failing inside FilterTable'
+    end
+    [{ 'matters' => 'it really does', 'another_filter' => 'example' }]
+  end
+
   def inside_matcher
     case @value
     when 'fail inside matcher'

test/unit/profiles/profile_resource_exceptions_test.rb

@@ -42,17 +42,17 @@ describe 'resource exception' do
   end
 
   describe 'within a matcher' do
-    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
-      checks[4][0][1][0].resource_skipped?.must_equal true
-      checks[4][0][1][0].resource_exception_message.must_equal 'Skipping inside matcher'
-      checks[4][0][1][0].resource_failed?.must_equal false
-    end
-
     it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
       checks[3][0][1][0].resource_failed?.must_equal true
       checks[3][0][1][0].resource_exception_message.must_equal 'Failing inside matcher'
       checks[3][0][1][0].resource_skipped?.must_equal false
     end
+
+    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+      checks[4][0][1][0].resource_skipped?.must_equal true
+      checks[4][0][1][0].resource_exception_message.must_equal 'Skipping inside matcher'
+      checks[4][0][1][0].resource_failed?.must_equal false
+    end
   end
 
   describe 'within a control' do
@@ -69,6 +69,50 @@ describe 'resource exception' do
     end
   end
 
+  describe 'within FilterTable' do
+    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+      checks[6][0][1][0].resource_skipped?.must_equal true
+      checks[6][0][1][0].resource_exception_message.must_equal 'Skipping inside FilterTable'
+      checks[6][0][1][0].resource_failed?.must_equal false
+    end
+
+    it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
+      checks[7][0][1][0].resource_failed?.must_equal true
+      checks[7][0][1][0].resource_exception_message.must_equal 'Failing inside FilterTable'
+      checks[7][0][1][0].resource_skipped?.must_equal false
+    end
+
+    describe 'and multiple filters are used' do
+      it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+        checks[8][0][1][0].resource_skipped?.must_equal true
+        checks[8][0][1][0].resource_exception_message.must_equal 'Skipping inside FilterTable'
+        checks[8][0][1][0].resource_failed?.must_equal false
+      end
+
+      it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
+        checks[9][0][1][0].resource_failed?.must_equal true
+        checks[9][0][1][0].resource_exception_message.must_equal 'Failing inside FilterTable'
+        checks[9][0][1][0].resource_skipped?.must_equal false
+      end
+
+      it 'does not halt the run/fail all tests when an incorrect filter is used' do
+        checks[10][0][1][0].resource_skipped?.must_equal true
+        checks[10][0][1][0].resource_exception_message.must_equal 'Skipping inside FilterTable'
+        checks[10][0][1][0].resource_failed?.must_equal false
+      end
+
+      it 'does not halt the run/fail all tests when an incorrect filter is used' do
+        checks[11][0][1][0].resource_failed?.must_equal true
+        checks[11][0][1][0].resource_exception_message.must_equal 'Failing inside FilterTable'
+        checks[11][0][1][0].resource_skipped?.must_equal false
+      end
+    end
+
+    it 'does not affect regular FilterTable usage' do
+      checks[12][0][1][0].another_filter.must_equal ['example']
+    end
+ end
+
   describe 'when using deprecated `resource_skip` method' do
     it 'warns the user' do
       _, err = capture_io { checks[0][0][1][0].resource_skipped }