From 598e8be07f1d712bd57cb5000e94d103970e4d0f Mon Sep 17 00:00:00 2001
From: Dominik Richter <dominik.richter@gmail.com>
Date: Tue, 5 Apr 2016 15:07:49 +0200
Subject: [PATCH] don't remove controls with only_if

instead mark them as skipped, but don't just remove them.

This also introduced a number of tests around only_if on the global level
---
 lib/inspec/profile_context.rb     | 39 +++++++++++-----------
 test/unit/profile_context_test.rb | 54 ++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/lib/inspec/profile_context.rb b/lib/inspec/profile_context.rb
index 211c404d7..f7c337bbb 100644
--- a/lib/inspec/profile_context.rb
+++ b/lib/inspec/profile_context.rb
@@ -102,6 +102,7 @@ module Inspec
         def initialize(backend, conf) # rubocop:disable Lint/NestedMethodDefinition, Lint/DuplicateMethods
           @backend = backend
           @conf = conf
+          @skip_profile = false
         end
 
         define_method :title do |arg|
@@ -115,20 +116,7 @@ module Inspec
         define_method :control do |*args, &block|
           id = args[0]
           opts = args[1] || {}
-
-          # Skip the control if the resource triggered a skip;
-          # However: when this is run on a mock backend, do not skip it.
-          # This is e.g. relevant for JSON generation, where we need all
-          # controls.
-          return if @skip_profile && os[:family] != 'unknown'
-
-          profile_context_owner.register_rule(rule_class.new(id, opts, &block))
-        end
-
-        alias_method :rule, :control
-
-        define_method :register_control do |control|
-          profile_context_owner.register_rule(control) unless control.nil?
+          register_control(rule_class.new(id, opts, &block))
         end
 
         define_method :describe do |*args, &block|
@@ -139,10 +127,24 @@ module Inspec
           rule = rule_class.new(id, {}) do
             res = describe(*args, &block)
           end
-          profile_context_owner.register_rule(rule, &block)
+          register_control(rule, &block)
           res
         end
 
+        define_method :register_control do |control, &block|
+          profile_context_owner.register_rule(control, &block) unless control.nil?
+
+          # Skip the control if the resource triggered a skip;
+          if @skip_profile
+            control.instance_variable_set(:@checks, [])
+            # TODO: we use os as the carrier here, but should consider
+            # a separate resource to do skipping
+            resource = os
+            resource.skip_resource('Skipped control due to only_if condition.')
+            control.describe(resource)
+          end
+        end
+
         # TODO: mock method for attributes; import attribute handling
         define_method :attributes do |_name, _options|
           nil
@@ -152,13 +154,14 @@ module Inspec
           profile_context_owner.unregister_rule(id)
         end
 
-        alias_method :skip_rule, :skip_control
-
         def only_if
           return unless block_given?
-          @skip_profile = !yield
+          @skip_profile ||= !yield
         end
 
+        alias_method :rule, :control
+        alias_method :skip_rule, :skip_control
+
         private
 
         def block_location(block, alternate_caller)
diff --git a/test/unit/profile_context_test.rb b/test/unit/profile_context_test.rb
index 806e017a6..dc544f7bb 100644
--- a/test/unit/profile_context_test.rb
+++ b/test/unit/profile_context_test.rb
@@ -117,7 +117,59 @@ describe Inspec::ProfileContext do
       load('expect(true).to_eq true').must_raise NoMethodError
     end
 
-    it 'provides the rule keyword in the global DSL' do
+    describe 'global only_if' do
+      let(:if_true) { "only_if { true }\n" }
+      let(:if_false) { "only_if { false }\n" }
+      let(:describe) { "describe nil do its(:to_i) { should eq rand } end\n" }
+      let(:control) { "control 1 do\n#{describe}end" }
+
+      it 'provides the keyword' do
+        profile.load(if_true)
+        profile.rules.must_equal({})
+      end
+
+      it 'doesnt affect controls when positive' do
+        profile.load(if_true + 'control 1')
+        profile.rules.values[0].must_be_kind_of Inspec::Rule
+      end
+
+      it 'doesnt remove controls when negative' do
+        profile.load(if_false + 'control 1')
+        profile.rules.values[0].must_be_kind_of Inspec::Rule
+      end
+
+      it 'alters controls when positive' do
+        profile.load(if_false + control)
+        get_checks.length.must_equal 1
+        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+      end
+
+      it 'alters non-controls when positive' do
+        profile.load(if_false + describe)
+        get_checks.length.must_equal 1
+        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+      end
+
+      it 'doesnt alter controls when negative' do
+        profile.load(if_true + control)
+        get_checks.length.must_equal 1
+        get_checks[0][1][0].must_be_nil
+      end
+
+      it 'doesnt alter non-controls when negative' do
+        profile.load(if_true + describe)
+        get_checks.length.must_equal 1
+        get_checks[0][1][0].must_be_nil
+      end
+    end
+
+    it 'provides the control keyword in the global DSL' do
+      profile.load('control 1')
+      profile.rules.keys.must_equal [1]
+      profile.rules.values[0].must_be_kind_of Inspec::Rule
+    end
+
+    it 'provides the rule keyword in the global DSL (legacy mode)' do
       profile.load('rule 1')
       profile.rules.keys.must_equal [1]
       profile.rules.values[0].must_be_kind_of Inspec::Rule