From 0d0ab0426c63305b47271bd6e0cb915e13f5c9a3 Mon Sep 17 00:00:00 2001 From: Tim Sharpe Date: Tue, 29 May 2018 23:04:40 +1000 Subject: [PATCH 1/4] (PDK-636) Make fixture cleaning optional --- lib/pdk/cli/test/unit.rb | 1 + lib/pdk/tests/unit.rb | 4 +-- spec/unit/pdk/cli/test/unit_spec.rb | 17 +++++++++ spec/unit/pdk/test/unit_spec.rb | 56 +++++++++++++++++++++++++++-- 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/lib/pdk/cli/test/unit.rb b/lib/pdk/cli/test/unit.rb index 449a2d075..432bab2bb 100644 --- a/lib/pdk/cli/test/unit.rb +++ b/lib/pdk/cli/test/unit.rb @@ -11,6 +11,7 @@ module PDK::CLI flag nil, :list, _('List all available unit test files.') flag nil, :parallel, _('Run unit tests in parallel.') flag :v, :verbose, _('More verbose output. Displays examples in each unit test file.') + flag :c, 'clean-fixtures', _('Clean up downloaded fixtures after the test run.') option nil, :tests, _('Specify a comma-separated list of unit test files to run.'), argument: :required, default: '' do |values| PDK::CLI::Util::OptionValidator.comma_separated_list?(values) diff --git a/lib/pdk/tests/unit.rb b/lib/pdk/tests/unit.rb index 9f1d2f0e3..4d95b96f7 100644 --- a/lib/pdk/tests/unit.rb +++ b/lib/pdk/tests/unit.rb @@ -7,7 +7,7 @@ module PDK module Test class Unit def self.cmd(tests, opts = {}) - rake_args = opts.key?(:parallel) ? 'parallel_spec' : 'spec' + rake_args = opts.key?(:parallel) ? 'parallel_spec_standalone' : 'spec_standalone' rake_args += "[#{tests}]" unless tests.nil? rake_args end @@ -89,7 +89,7 @@ def self.invoke(report, options = {}) result[:exit_code] ensure - tear_down + tear_down if options[:'clean-fixtures'] end def self.parse_output(report, json_data) diff --git a/spec/unit/pdk/cli/test/unit_spec.rb b/spec/unit/pdk/cli/test/unit_spec.rb index 21098abcd..3fac73fea 100644 --- a/spec/unit/pdk/cli/test/unit_spec.rb +++ b/spec/unit/pdk/cli/test/unit_spec.rb @@ -89,6 +89,23 @@ allow(PDK::Report).to receive(:new).and_return(reporter) end + context 'when passed --clean-fixtures' do + it 'invokes the command with :clean-fixtures => true' do + expect(PDK::Test::Unit).to receive(:invoke).with(reporter, :tests => anything, :'clean-fixtures' => true).once.and_return(0) + expect { + test_unit_cmd.run_this(['--clean-fixtures']) + }.to exit_zero + end + end + + context 'when not passed --clean-fixtures' do + it 'invokes the command without :clean-fixtures' do + expect(PDK::Test::Unit).to receive(:invoke).with(reporter, tests: anything).once.and_return(0) + expect { + test_unit_cmd.run_this([]) + }.to exit_zero + end + end context 'when tests pass' do before(:each) do expect(PDK::Test::Unit).to receive(:invoke).with(reporter, hash_including(:tests)).once.and_return(0) diff --git a/spec/unit/pdk/test/unit_spec.rb b/spec/unit/pdk/test/unit_spec.rb index d4fe74d5a..ac065315a 100644 --- a/spec/unit/pdk/test/unit_spec.rb +++ b/spec/unit/pdk/test/unit_spec.rb @@ -168,7 +168,7 @@ it 'uses the parallel_spec rake task' do cmd = described_class.cmd(nil, parallel: true) - expect(cmd).to eq('parallel_spec') + expect(cmd).to eq('parallel_spec_standalone') end end @@ -176,7 +176,7 @@ it 'uses the spec rake task' do cmd = described_class.cmd(nil) - expect(cmd).to eq('spec') + expect(cmd).to eq('spec_standalone') end end @@ -184,7 +184,7 @@ it 'passes file paths to rake' do cmd = described_class.cmd('/path/to/test1,/path/to/test2') - expect(cmd).to eq('spec[/path/to/test1,/path/to/test2]') + expect(cmd).to eq('spec_standalone[/path/to/test1,/path/to/test2]') end end end @@ -301,6 +301,56 @@ end end + context 'configurable fixture cleaning' do + after(:each) do + described_class.invoke(report, options) + end + + let(:rspec_json_output) do + '{ + "examples": + [ + { + "id": "./spec/fixtures/modules/testmod/spec/classes/testmod_spec.rb[1:3:1]", + "status": "passed", + "pending_message": null + } + ], + "summary": { + "duration": 0.295112, + "example_count": 1, + "failure_count": 0, + "pending_count": 0 + } + }' + end + + context 'when enabled' do + let(:options) do + { + :'clean-fixtures' => true, + :tests => 'testmod_spec.rb', + } + end + + it 'calls tear_down' do + expect(described_class).to receive(:tear_down) + end + end + + context 'when disabled' do + let(:options) do + { + tests: 'testmod_spec.rb', + } + end + + it 'does not call tear_down' do + expect(described_class).not_to receive(:tear_down) + end + end + end + context 'in parallel without examples' do let(:rspec_json_output) do '{ From 8d1c1ee76a1c148cb42a8bae0f381152946829c7 Mon Sep 17 00:00:00 2001 From: Tim Sharpe Date: Tue, 5 Jun 2018 15:03:27 +1000 Subject: [PATCH 2/4] (maint) Tear down fixtures if fixture setup fails --- lib/pdk/tests/unit.rb | 2 ++ spec/unit/pdk/test/unit_spec.rb | 1 + 2 files changed, 3 insertions(+) diff --git a/lib/pdk/tests/unit.rb b/lib/pdk/tests/unit.rb index 4d95b96f7..7b20bd022 100644 --- a/lib/pdk/tests/unit.rb +++ b/lib/pdk/tests/unit.rb @@ -57,6 +57,8 @@ def self.setup return if result[:exit_code].zero? + tear_down + PDK.logger.error(_('The spec_prep rake task failed with the following error(s):')) print_failure(result, _('Failed to prepare to run the unit tests.')) end diff --git a/spec/unit/pdk/test/unit_spec.rb b/spec/unit/pdk/test/unit_spec.rb index ac065315a..697936041 100644 --- a/spec/unit/pdk/test/unit_spec.rb +++ b/spec/unit/pdk/test/unit_spec.rb @@ -79,6 +79,7 @@ expect($stderr).to receive(:puts).with('some output') expect($stderr).to receive(:puts).with('some error') expect(logger).to receive(:error).with(a_string_matching(%r{spec_prep rake task failed})) + expect(described_class).to receive(:tear_down) expect { described_class.setup From d90434f5dd08d1a2adadd4feb90aefdea9a08134 Mon Sep 17 00:00:00 2001 From: Tim Sharpe Date: Tue, 5 Jun 2018 15:03:48 +1000 Subject: [PATCH 3/4] (maint) Fix up unit tests --- spec/acceptance/test_unit_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/acceptance/test_unit_spec.rb b/spec/acceptance/test_unit_spec.rb index 2289630cc..bc06f1eea 100644 --- a/spec/acceptance/test_unit_spec.rb +++ b/spec/acceptance/test_unit_spec.rb @@ -18,7 +18,6 @@ its(:stderr) { is_expected.to match(%r{running unit tests}i) } its(:stderr) { is_expected.to match(%r{no examples found}i) } its(:stderr) { is_expected.to match(%r{evaluated 0 tests}i) } - its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) } end describe command('pdk test unit --parallel') do @@ -27,7 +26,6 @@ its(:stderr) { is_expected.to match(%r{running unit tests in parallel}i) } its(:stderr) { is_expected.to match(%r{no examples found}i) } its(:stderr) { is_expected.to match(%r{evaluated 0 tests}i) } - its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) } end end @@ -63,14 +61,12 @@ its(:exit_status) { is_expected.to eq(0) } its(:stderr) { is_expected.to match(%r{preparing to run the unit tests}i) } its(:stderr) { is_expected.to match(%r{running unit tests.*4 tests.*0 failures}im) } - its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) } end describe command('pdk test unit --parallel') do its(:exit_status) { is_expected.to eq(0) } its(:stderr) { is_expected.to match(%r{preparing to run the unit tests}i) } its(:stderr) { is_expected.to match(%r{running unit tests in parallel.*4 tests.*0 failures}im) } - its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) } end end From ba33a291f4fbac1ecce4178aa65104339c6a4647 Mon Sep 17 00:00:00 2001 From: Tim Sharpe Date: Tue, 5 Jun 2018 17:36:54 +1000 Subject: [PATCH 4/4] (maint) Clean up test fixtures before building package --- lib/pdk/module/build.rb | 10 ++++++++++ spec/unit/pdk/module/build_spec.rb | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/lib/pdk/module/build.rb b/lib/pdk/module/build.rb index 2029814db..f995bee29 100644 --- a/lib/pdk/module/build.rb +++ b/lib/pdk/module/build.rb @@ -3,6 +3,7 @@ require 'zlib' require 'pathspec' require 'find' +require 'pdk/tests/unit' module PDK module Module @@ -36,6 +37,7 @@ def package_file # # @return [String] The path to the built package file. def build + cleanup_module create_build_dir stage_module_in_build_dir @@ -81,6 +83,14 @@ def cleanup_build_dir FileUtils.rm_rf(build_dir, secure: true) end + # Clean up any files created during use of the PDK that shouldn't be part + # of the built module (e.g. test fixtures). + # + # @return nil + def cleanup_module + PDK::Test::Unit.tear_down + end + # Combine the module name and version into a Forge-compatible dash # separated string. # diff --git a/spec/unit/pdk/module/build_spec.rb b/spec/unit/pdk/module/build_spec.rb index 19a822a11..455ec9ed4 100644 --- a/spec/unit/pdk/module/build_spec.rb +++ b/spec/unit/pdk/module/build_spec.rb @@ -4,6 +4,10 @@ describe PDK::Module::Build do subject { described_class.new(initialize_options) } + before(:each) do + allow(PDK::Test::Unit).to receive(:tear_down) + end + let(:initialize_options) { {} } let(:root_dir) { Gem.win_platform? ? 'C:/' : '/' }