From f5ec7c9c65b1dd17bedde4d30214bf2e280b736c Mon Sep 17 00:00:00 2001 From: Jerry Aldrich Date: Mon, 14 Jan 2019 10:56:01 -0800 Subject: [PATCH] Fix style/RuboCop on `cw/deprecation-facility` Signed-off-by: Jerry Aldrich --- docs/dev/deprecation.md | 11 +++-- lib/utils/deprecation.rb | 2 +- lib/utils/deprecation/config_file.rb | 12 ++---- lib/utils/deprecation/deprecator.rb | 1 - test/functional/logging_test.rb | 37 +++++++--------- .../deprecation/resource_pack/README.md | 4 +- .../libraries/deprecation_tester.rb | 33 +++++++-------- .../deprecation/typical/controls/typical.rb | 4 +- test/unit/utils/deprecation_test.rb | 42 +++++++++---------- 9 files changed, 65 insertions(+), 81 deletions(-) diff --git a/docs/dev/deprecation.md b/docs/dev/deprecation.md index cc06ba6cd..1b0c90e3c 100644 --- a/docs/dev/deprecation.md +++ b/docs/dev/deprecation.md @@ -9,13 +9,13 @@ The most important goal of the deprecation facility was to collect decisions abo ### Example ``` -# In some code in Inspec core +# In some code in InSpec core def something_crufty Inspec.deprecate :old_feature, 'Don't call something_crufty anymore' end ``` -If that gets called, inspec will consult the deprecation configuration, and then execute one of four actions: warn, fail_control, exit, or ignore. +If that gets called, `inspec` will consult the deprecation configuration, and then execute one of four actions: warn, fail_control, exit, or ignore. ## Concepts @@ -48,7 +48,7 @@ For v4: } ``` -Now, a warning (fed through Inspec::Log.warn) will appear each time the property is accessed, but it will otherwise behave normally. +Now, a warning (fed through `Inspec::Log.warn`) will appear each time the property is accessed, but it will otherwise behave normally. For v5, you have some choices as to how to harden the deprecation. `fail_control` will fail any control the deprecation is used in; while `exit` will exit the inspec run entirely. @@ -61,7 +61,6 @@ For v5: Again, no need to update the deprecation calls; though in v6 it might make sense to remove the old code entirely. - ### Groups When you make a deprecation message, you add it to a group. InSpec will read the file `$INSPEC_INSTALL_ROOT/etc/deprecation.json` to determine the known groups - you may add to that file at any time. @@ -84,7 +83,7 @@ For all actions except `ignore`, a message is assembled, consisting of: #### exit -Issues an ERROR via Inspec::Log.error with the assembled message, then immediately exits the process via Inspec::UI. The reporters are not executed. By default, the exit code will be 3; but you can set a different code using the `exit_status` property of the group in the config file. +Issues an ERROR via `Inspec::Log.error` with the assembled message, then immediately exits the process via Inspec::UI. The reporters are not executed. By default, the exit code will be 3; but you can set a different code using the `exit_status` property of the group in the config file. #### fail_control @@ -98,4 +97,4 @@ Does nothing; this is very useful for staging deprecations - you can have the de #### warn -Issues a WARN to the Inspec::Log.warn facility with the assembled message. +Issues a WARN to the `Inspec::Log.warn` facility with the assembled message. diff --git a/lib/utils/deprecation.rb b/lib/utils/deprecation.rb index 2f12e4cca..1834347be 100644 --- a/lib/utils/deprecation.rb +++ b/lib/utils/deprecation.rb @@ -1,4 +1,4 @@ -# A system to provide a unified deprecation facility for inspec +# A system to provide a unified deprecation facility for InSpec require 'utils/deprecation/errors' require 'utils/deprecation/config_file' diff --git a/lib/utils/deprecation/config_file.rb b/lib/utils/deprecation/config_file.rb index 0c5597276..db68dce30 100644 --- a/lib/utils/deprecation/config_file.rb +++ b/lib/utils/deprecation/config_file.rb @@ -9,14 +9,10 @@ module Inspec # What actions may you specify to be taken when a deprecation is encountered? VALID_ACTIONS = [ - # Hard exit inspec, no stacktrace, exit code specified or DEFAULT_HARD_EXIT_STATUS - :exit, - # Fail the control with the message. If not in a control, do :warn action instead. - :fail_control, - # Do nothing. - :ignore, - # Issue a warning - :warn, + :exit, # Hard exit `inspec`, no stacktrace, exit code specified or DEFAULT_HARD_EXIT_STATUS + :fail_control, # Fail the control with a message. If not in a control, do :warn action instead. + :ignore, # Do nothing. + :warn, # Issue a warning ].freeze VALID_GROUP_FIELDS = %w{action suffix prefix exit_status}.freeze diff --git a/lib/utils/deprecation/deprecator.rb b/lib/utils/deprecation/deprecator.rb index e0764b315..0c181c830 100644 --- a/lib/utils/deprecation/deprecator.rb +++ b/lib/utils/deprecation/deprecator.rb @@ -1,4 +1,3 @@ - require 'utils/deprecation/config_file' require 'inspec/log' diff --git a/test/functional/logging_test.rb b/test/functional/logging_test.rb index f25c18fc6..27cd2c1da 100644 --- a/test/functional/logging_test.rb +++ b/test/functional/logging_test.rb @@ -1,7 +1,6 @@ # Logging and deprecation facilities functional tests require 'functional/helper' -require 'byebug' describe 'Deprecation Facility Behavior' do include FunctionalHelper @@ -9,19 +8,18 @@ describe 'Deprecation Facility Behavior' do let(:profile) { File.join(profile_path, 'deprecation', profile_name) } let(:invocation) { "exec #{profile} #{control_flag}" } # Running in JSON mode has the side-effect of sending log messages to STDERR - let(:run_result) { run_inspec_process(invocation, json: true)} + let(:run_result) { run_inspec_process(invocation, json: true) } # Expect one control, 3 results let(:json_result) { run_result.payload.json['profiles'][0]['controls'][0]['results'] } describe 'when the deprecation is in a custom resource and the deprecate DSL method is used' do describe 'when the action is to fail the control' do - describe 'when the resource is called in a control' do let(:profile_name) { 'typical' } let(:control_flag) { '--controls deprecate_fail_mode' } - it "should result in a failed control" do + it 'should result in a failed control' do run_result.stderr.must_be_empty run_result.exit_status.must_equal 100 json_result.count.must_equal 3 @@ -40,7 +38,7 @@ describe 'Deprecation Facility Behavior' do let(:profile_name) { 'bare' } let(:control_flag) { '' } - it "should result in a warning, not a stacktrace or abort" do + it 'should result in a warning, not a stacktrace or abort' do run_result.exit_status.must_equal 0 json_result.count.must_equal 1 json_result[0]['status'].must_equal 'passed' @@ -55,7 +53,6 @@ describe 'Deprecation Facility Behavior' do deprecation_line.must_include '(used at' deprecation_line.must_include 'test/unit/mock/profiles/deprecation/bare/controls/bare.rb' deprecation_line.must_include 'bare.rb:4' - end end end @@ -64,7 +61,7 @@ describe 'Deprecation Facility Behavior' do let(:profile_name) { 'typical' } let(:control_flag) { '--controls deprecate_exit_mode_implicit' } - it "should result in an exit with a special code" do + it 'should result in an exit with a special code' do # 3 is the FATAL_DEPRECATION value from Inspec::UI run_result.exit_status.must_equal 3 @@ -77,13 +74,12 @@ describe 'Deprecation Facility Behavior' do deprecation_line.must_include 'DEPRECATION' deprecation_line.must_include 'ERROR' deprecation_line.must_include 'This should exit' - deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator + deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator deprecation_line.must_include 'test/unit/mock/profiles/deprecation/typical/controls/typical.rb' # Frame should have been identified as coming from the test profile deprecation_line.must_include 'typical.rb:29' # Line number check # The reporter should not fire run_result.stdout.must_be_empty - end end @@ -91,7 +87,7 @@ describe 'Deprecation Facility Behavior' do let(:profile_name) { 'typical' } let(:control_flag) { '--controls deprecate_exit_mode_explicit' } - it "should result in an exit with a special code" do + it 'should result in an exit with a special code' do # 8 is a custom value run_result.exit_status.must_equal 8 @@ -102,9 +98,9 @@ describe 'Deprecation Facility Behavior' do # Contents of the deprecation deprecation_line = stderr_lines.first deprecation_line.must_include 'DEPRECATION' # Flagged as deprecation - deprecation_line.must_include 'ERROR' # Flagged as an error + deprecation_line.must_include 'ERROR' # Flagged as an error deprecation_line.must_include 'This should exit' # Specific deprecation message - deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator + deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator deprecation_line.must_include 'test/unit/mock/profiles/deprecation/typical/controls/typical.rb' # Frame should have been identified as coming from the test profile deprecation_line.must_include 'typical.rb:46' # Line number check @@ -117,7 +113,7 @@ describe 'Deprecation Facility Behavior' do let(:profile_name) { 'typical' } let(:control_flag) { '--controls deprecate_warn_mode' } - it "should result in a warning, not a stacktrace or abort" do + it 'should result in a warning, not a stacktrace or abort' do run_result.exit_status.must_equal 0 json_result.count.must_equal 3 json_result[0]['status'].must_equal 'passed' @@ -127,14 +123,14 @@ describe 'Deprecation Facility Behavior' do stderr_lines = run_result.stderr.split("\n") stderr_lines.count.must_equal 1 + # Content of the deprecation deprecation_line = stderr_lines.first - deprecation_line.must_include 'DEPRECATION' - deprecation_line.must_include 'WARN' - deprecation_line.must_include 'This should warn' - deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator + deprecation_line.must_include 'DEPRECATION' # Flagged as deprecation + deprecation_line.must_include 'WARN' # Flagged as a warning + deprecation_line.must_include 'This should warn' # Specific deprecation message + deprecation_line.must_include '(used at' # Beginning of a single-frame stack locator deprecation_line.must_include 'test/unit/mock/profiles/deprecation/typical/controls/typical.rb' # Frame should have been identified as coming from the test profile deprecation_line.must_include 'typical.rb:63' # Line number check - end end @@ -142,7 +138,7 @@ describe 'Deprecation Facility Behavior' do let(:profile_name) { 'typical' } let(:control_flag) { '--controls deprecate_ignore_mode' } - it "should appear to be a normal run, no warnings or stacktrace or abort" do + it 'should appear to be a normal run, no warnings or stacktrace or abort' do run_result.exit_status.must_equal 0 json_result.count.must_equal 3 json_result[0]['status'].must_equal 'passed' @@ -150,8 +146,7 @@ describe 'Deprecation Facility Behavior' do json_result[2]['status'].must_equal 'passed' run_result.stderr.must_be_empty - end end end -end \ No newline at end of file +end diff --git a/test/unit/mock/profiles/deprecation/resource_pack/README.md b/test/unit/mock/profiles/deprecation/resource_pack/README.md index d3466ff95..51694bff6 100644 --- a/test/unit/mock/profiles/deprecation/resource_pack/README.md +++ b/test/unit/mock/profiles/deprecation/resource_pack/README.md @@ -1,5 +1,5 @@ # Profile for use with deprecation facility -This profile defines a custom resource which uses the Inspec deprecation facility to issue deprecation actions in several different circumstances. +This profile defines a custom resource which uses the InSpec deprecation facility to issue deprecation actions in several different circumstances. -See test/functional/logging_test.rb for details. \ No newline at end of file +See test/functional/logging_test.rb for details. diff --git a/test/unit/mock/profiles/deprecation/resource_pack/libraries/deprecation_tester.rb b/test/unit/mock/profiles/deprecation/resource_pack/libraries/deprecation_tester.rb index 9c887cd75..26b7addae 100644 --- a/test/unit/mock/profiles/deprecation/resource_pack/libraries/deprecation_tester.rb +++ b/test/unit/mock/profiles/deprecation/resource_pack/libraries/deprecation_tester.rb @@ -1,62 +1,59 @@ - require 'stringio' class DeprecationTester < Inspec.resource(1) name :deprecation_tester DEPRECATION_CFG = <<~EOC - { - "file_version": "1.0.0", - "unknown_group_action": "warn", - "groups": { - "a_group_that_will_warn" : { "action": "warn" }, - "a_group_that_will_exit" : { "action": "exit" }, - "a_group_that_will_exit_with_a_code" : { "action": "exit", "exit_status": 8 }, - "an_ignored_group" : { "action": "ignore" }, - "a_group_that_will_fail" : { "action": "fail_control" } + { + "file_version": "1.0.0", + "unknown_group_action": "warn", + "groups": { + "a_group_that_will_warn": { "action": "warn" }, + "a_group_that_will_exit": { "action": "exit" }, + "a_group_that_will_exit_with_a_code": { "action": "exit", "exit_status": 8 }, + "an_ignored_group": { "action": "ignore" }, + "a_group_that_will_fail": { "action": "fail_control" } + } } - } EOC def fail_me Inspec::Deprecation::Deprecator.class_test_cfg_io(StringIO.new(DEPRECATION_CFG)) - #deprecate(:a_group_that_will_fail, 'This should fail') Inspec.deprecate(:a_group_that_will_fail, 'This should fail') + 'fail_me_return_value' end def exit_me_default_code Inspec::Deprecation::Deprecator.class_test_cfg_io(StringIO.new(DEPRECATION_CFG)) - #deprecate(:a_group_that_will_exit, 'This should exit') Inspec.deprecate(:a_group_that_will_exit, 'This should exit') + 'exit_me_return_value' end def exit_me_explicit_code Inspec::Deprecation::Deprecator.class_test_cfg_io(StringIO.new(DEPRECATION_CFG)) - #deprecate(:a_group_that_will_exit_with_a_code, 'This should exit') Inspec.deprecate(:a_group_that_will_exit_with_a_code, 'This should exit') + 'exit_me_return_value' end def ignore_me Inspec::Deprecation::Deprecator.class_test_cfg_io(StringIO.new(DEPRECATION_CFG)) - #deprecate(:an_ignored_group, 'This should be ignored') Inspec.deprecate(:an_ignored_group, 'This should be ignored') + 'ignore_me_return_value' end def warn_me Inspec::Deprecation::Deprecator.class_test_cfg_io(StringIO.new(DEPRECATION_CFG)) - #deprecate(:a_group_that_will_warn, 'This should warn') Inspec.deprecate(:a_group_that_will_warn, 'This should warn') + 'warn_me_return_value' end - end - diff --git a/test/unit/mock/profiles/deprecation/typical/controls/typical.rb b/test/unit/mock/profiles/deprecation/typical/controls/typical.rb index 168408cb4..5fdfbee4d 100644 --- a/test/unit/mock/profiles/deprecation/typical/controls/typical.rb +++ b/test/unit/mock/profiles/deprecation/typical/controls/typical.rb @@ -1,6 +1,6 @@ # encoding: utf-8 -# Notice: line numbers are sensitive! They are used for stacktrace testing in functional/loggin_test.rb +# Notice: line numbers are sensitive! They are used for stacktrace testing in functional/logging_test.rb control 'deprecate_fail_mode' do @@ -83,4 +83,4 @@ control 'deprecate_ignore_mode' do it { should include 'test-03' } end -end \ No newline at end of file +end diff --git a/test/unit/utils/deprecation_test.rb b/test/unit/utils/deprecation_test.rb index b665e901f..ed5972ecd 100644 --- a/test/unit/utils/deprecation_test.rb +++ b/test/unit/utils/deprecation_test.rb @@ -1,7 +1,6 @@ require 'minitest' require 'minitest/spec' require 'stringio' -require 'byebug' require 'utils/deprecation' @@ -15,7 +14,7 @@ describe 'The global deprecation method' do end it 'must take two required and one optional arg' do # See http://ruby-doc.org/core-2.5.3/Method.html#method-i-arity - Inspec.method(:deprecate).arity.must_equal -3 + Inspec.method(:deprecate).arity.must_equal(-3) end end end @@ -38,7 +37,7 @@ describe 'The deprecation config file object' do describe 'when the file version is missing' do let(:cfg_fixture) { :missing_file_version } it 'should throw an InvalidConfigFileError' do - ex = assert_raises (Inspec::Deprecation::InvalidConfigFileError) { config_file } + ex = assert_raises(Inspec::Deprecation::InvalidConfigFileError) { config_file } ex.message.must_include 'Missing file_version field' end end @@ -46,7 +45,7 @@ describe 'The deprecation config file object' do describe 'when the file version is unsupported' do let(:cfg_fixture) { :bad_file_version } it 'should throw an InvalidConfigFileError' do - ex = assert_raises (Inspec::Deprecation::InvalidConfigFileError) { config_file } + ex = assert_raises(Inspec::Deprecation::InvalidConfigFileError) { config_file } ex.message.must_include 'Unrecognized file_version' # message ex.message.must_include '1.0.0' # version that IS supported ex.message.must_include '99.99.99' # version that was seen @@ -56,7 +55,7 @@ describe 'The deprecation config file object' do describe 'when the groups entry is not a hash' do let(:cfg_fixture) { :groups_not_hash } it 'should throw an InvalidConfigFileError' do - ex = assert_raises (Inspec::Deprecation::InvalidConfigFileError) { config_file } + ex = assert_raises(Inspec::Deprecation::InvalidConfigFileError) { config_file } ex.message.must_include 'Groups field must be a Hash' # message end end @@ -64,7 +63,7 @@ describe 'The deprecation config file object' do describe 'when a group entry has an unrecognized action' do let(:cfg_fixture) { :bad_group_action } it 'should throw an UnrecognizedActionError' do - ex = assert_raises (Inspec::Deprecation::UnrecognizedActionError) { config_file } + ex = assert_raises(Inspec::Deprecation::UnrecognizedActionError) { config_file } ex.message.must_include 'Unrecognized action' # message ex.message.must_include 'methane_pockets' # offending group name ex.message.must_include 'ignore' # an action that IS supported @@ -78,7 +77,7 @@ describe 'The deprecation config file object' do describe 'when a group entry has an unrecognized field' do let(:cfg_fixture) { :bad_group_field } it 'should throw an InvalidConfigError' do - ex = assert_raises (Inspec::Deprecation::InvalidConfigFileError) { config_file } + ex = assert_raises(Inspec::Deprecation::InvalidConfigFileError) { config_file } ex.message.must_include 'Unrecognized field' # message ex.message.must_include 'pansporia' # offending group name ex.message.must_include 'action' # a field that IS supported @@ -110,10 +109,10 @@ describe 'The Deprecator object' do let(:cfg_fixture) { :basic } describe 'when it has no args' do - it 'should create an object with basic ' do + it 'should create an object with basic' do dpcr = Inspec::Deprecation::Deprecator.new dpcr.must_respond_to(:handle_deprecation) - # more? + # TODO: more? end end @@ -143,25 +142,24 @@ describe 'The Deprecator object' do end end - # TODO - stack analysis + # TODO: stack analysis # in_control? - # TODO - anything else here? + # TODO: anything else here? end - module DeprecationTestHelper class Config FIXTURES = { basic: <<~EOC0, - { - "file_version": "1.0.0", "unknown_group_action": "ignore", - "groups": { - "a_group_that_will_warn" : { "action": "warn", "suffix": "Did you know chickens are dinosaurs?" }, - "a_group_that_will_exit" : { "action": "exit", "exit_status": 8, "prefix": "No thanks!" }, - "an_ignored_group" : { "action": "ignore" }, - "a_group_that_will_fail" : { "action": "fail_control" } + { + "file_version": "1.0.0", "unknown_group_action": "ignore", + "groups": { + "a_group_that_will_warn" : { "action": "warn", "suffix": "Did you know chickens are dinosaurs?" }, + "a_group_that_will_exit" : { "action": "exit", "exit_status": 8, "prefix": "No thanks!" }, + "an_ignored_group" : { "action": "ignore" }, + "a_group_that_will_fail" : { "action": "fail_control" } + } } - } EOC0 missing_file_version: '{ "unknown_group_action": "ignore", "groups": {} }', bad_file_version: '{ "file_version": "99.99.99", "unknown_group_action": "ignore", "groups": {} }', @@ -169,10 +167,10 @@ module DeprecationTestHelper empty: '{ "file_version": "1.0.0", "groups": {} }', bad_group_action: '{ "file_version": "1.0.0", "groups": { "methane_pockets" : { "action": "explode" } } }', bad_group_field: '{ "file_version": "1.0.0", "groups": { "pansporia" : { "martian": "yes" } } }', - } + }.freeze def self.get_io_for_fixture(fixture) StringIO.new(FIXTURES[fixture]) end end -end \ No newline at end of file +end