Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDK-636) Make fixture cleaning optional #515

Merged
merged 4 commits into from
Jun 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/pdk/cli/test/unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions lib/pdk/module/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'zlib'
require 'pathspec'
require 'find'
require 'pdk/tests/unit'

module PDK
module Module
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
#
Expand Down
6 changes: 4 additions & 2 deletions lib/pdk/tests/unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -89,7 +91,7 @@ def self.invoke(report, options = {})

result[:exit_code]
ensure
tear_down
tear_down if options[:'clean-fixtures']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users forgetting tear-down and then running pdk build would end up bundling their fixtures. Until pdk release prep can clean the fixtures, I would suggest having pdk build run a spec_clean or tear_down or whatever to avoid bundling fixtures.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hunner good call!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodjek has this been ticketed so we don't forget to update pdk build? I'm wondering if we should add the fixtures directory to the .pdkignore vs calling something that will change a tagged repo/directory before building.

end

def self.parse_output(report, json_data)
Expand Down
4 changes: 0 additions & 4 deletions spec/acceptance/test_unit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions spec/unit/pdk/cli/test/unit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions spec/unit/pdk/module/build_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:/' : '/' }

Expand Down
57 changes: 54 additions & 3 deletions spec/unit/pdk/test/unit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -168,23 +169,23 @@
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

context 'when run without the parallel option' do
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

context 'when run with tests option' do
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
Expand Down Expand Up @@ -301,6 +302,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
'{
Expand Down