diff --git a/Changelog.md b/Changelog.md index 59a94653..870ed18f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +# v0.8.23 2018-12-04 + +* Errors between isolation and tests do not kill mutations anymore. + # v0.8.22 2018-12-04 * Remove hard ruby version requirement. 2.5 is still the only officially supported version. diff --git a/lib/mutant.rb b/lib/mutant.rb index 22c9f538..e5c36ae2 100644 --- a/lib/mutant.rb +++ b/lib/mutant.rb @@ -197,6 +197,7 @@ require 'mutant/reporter/cli/printer' require 'mutant/reporter/cli/printer/config' require 'mutant/reporter/cli/printer/env_progress' require 'mutant/reporter/cli/printer/env_result' +require 'mutant/reporter/cli/printer/isolation_result' require 'mutant/reporter/cli/printer/mutation_progress_result' require 'mutant/reporter/cli/printer/mutation_result' require 'mutant/reporter/cli/printer/status' diff --git a/lib/mutant/env.rb b/lib/mutant/env.rb index a66859c5..8abb0a13 100644 --- a/lib/mutant/env.rb +++ b/lib/mutant/env.rb @@ -24,10 +24,12 @@ module Mutant # # @return [Result::Mutation] def kill(mutation) - test_result = run_mutation_tests(mutation) + start = Timer.now + Result::Mutation.new( - mutation: mutation, - test_result: test_result + isolation_result: run_mutation_tests(mutation), + mutation: mutation, + runtime: Timer.now - start ) end @@ -48,24 +50,12 @@ module Mutant # @param [Isolation] isolation # @param [Integration] integration # - # @return [Result::Test] - # - # rubocop:disable MethodLength + # @return [Result::Isolation] def run_mutation_tests(mutation) - start = Timer.now - tests = selections.fetch(mutation.subject) - config.isolation.call do mutation.insert(config.kernel) - integration.call(tests) + integration.call(selections.fetch(mutation.subject)) end - rescue Isolation::Error => error - Result::Test.new( - output: error.message, - passed: false, - runtime: Timer.now - start, - tests: tests - ) end end # Env diff --git a/lib/mutant/isolation.rb b/lib/mutant/isolation.rb index ad6f5a0e..fc9bad4a 100644 --- a/lib/mutant/isolation.rb +++ b/lib/mutant/isolation.rb @@ -1,12 +1,36 @@ # frozen_string_literal: true module Mutant + # Isolation mechanism class Isolation include AbstractType + # Isolated computation result + class Result + include AbstractType + + abstract_method :value + abstract_method :error + + # Test for success + # + # @return [Boolean] + def success? + instance_of?(Success) + end + + class Success < self + include Concord::Public.new(:value) + end # Success + + class Error < self + include Concord::Public.new(:error) + end # Error + end # Result + # Call block in isolation # - # @return [Object] + # @return [Result] # the blocks result abstract_method :call end # Isolation diff --git a/lib/mutant/isolation/fork.rb b/lib/mutant/isolation/fork.rb index ef6a76ad..35ec147c 100644 --- a/lib/mutant/isolation/fork.rb +++ b/lib/mutant/isolation/fork.rb @@ -3,10 +3,6 @@ module Mutant class Isolation # Isolation via the fork(2) systemcall. - # - # We do inject so many globals and common patterns to make this unit - # specifiable without mocking the globals and more important: Not having - # mutations that bypass mocks into a real world side effect. class Fork < self include Anima.new(:process, :stderr, :stdout, :io, :devnull, :marshal) @@ -15,17 +11,14 @@ module Mutant # Call block in isolation # - # @return [Object] - # returns block execution result - # - # @raise [Error] - # if block terminates abnormal + # @return [Result] + # execution result def call(&block) io.pipe(binmode: true) do |pipes| parent(*pipes, &block) end rescue => exception - raise Error, exception + Result::Error.new(exception) end # Handle parent process @@ -40,9 +33,10 @@ module Mutant end writer.close - marshal.load(reader) - ensure - process.waitpid(pid) if pid + + Result::Success.new(marshal.load(reader)).tap do + process.waitpid(pid) + end end # Handle child process diff --git a/lib/mutant/isolation/none.rb b/lib/mutant/isolation/none.rb index d4b16504..d184f562 100644 --- a/lib/mutant/isolation/none.rb +++ b/lib/mutant/isolation/none.rb @@ -3,8 +3,6 @@ module Mutant # Module providing isolation class Isolation - Error = Class.new(RuntimeError) - # Absolutly no isolation # # Only useful for debugging. @@ -12,14 +10,13 @@ module Mutant # Call block in no isolation # - # @return [Object] + # @return [Result] # - # @raise [Error] - # if block terminates abnormal + # ignore :reek:UtilityFunction def call - yield + Result::Success.new(yield) rescue => exception - raise Error, exception + Result::Error.new(exception) end end # None diff --git a/lib/mutant/reporter/cli/printer/isolation_result.rb b/lib/mutant/reporter/cli/printer/isolation_result.rb new file mode 100644 index 00000000..2850b499 --- /dev/null +++ b/lib/mutant/reporter/cli/printer/isolation_result.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Mutant + class Reporter + class CLI + class Printer + # Reporter for mutation results + # + # :reek:TooManyConstants + class IsolationResult < self + ERROR_MESSAGE = <<~'MESSAGE' + Killing the mutation resulted in an integration error. + This is the case when the tests selected for the current mutation + did not produce a test result, but instead an exception was raised. + + This may point to the following problems: + * Bug in mutant + * Bug in the ruby interpreter + * Bug in your test suite + * Bug in your test suite under concurrency + + The following exception was raised: + + ``` + %s + %s + ``` + MESSAGE + + private_constant(*constants(false)) + + # Run report printer + # + # @return [undefined] + def run + if object.success? + visit(TestResult, object.value) + else + visit_error_result + end + end + + private + + # Visit failed test results + # + # @return [undefined] + def visit_error_result + exception = object.error + + puts( + ERROR_MESSAGE % [ + exception.inspect, + exception.backtrace.join("\n") + ] + ) + end + end # IsolationResult + end # Printer + end # CLI + end # Reporter +end # Mutant diff --git a/lib/mutant/reporter/cli/printer/mutation_result.rb b/lib/mutant/reporter/cli/printer/mutation_result.rb index 81ab5127..f21643c7 100644 --- a/lib/mutant/reporter/cli/printer/mutation_result.rb +++ b/lib/mutant/reporter/cli/printer/mutation_result.rb @@ -9,7 +9,7 @@ module Mutant # :reek:TooManyConstants class MutationResult < self - delegate :mutation, :test_result + delegate :mutation, :isolation_result MAP = { Mutant::Mutation::Evil => :evil_details, @@ -101,7 +101,7 @@ module Mutant # @return [String] def noop_details info(NOOP_MESSAGE) - visit_test_result + visit_isolation_result end # Neutral details @@ -109,14 +109,14 @@ module Mutant # @return [String] def neutral_details info(NEUTRAL_MESSAGE, original_node.inspect, mutation.source) - visit_test_result + visit_isolation_result end # Visit failed test results # # @return [undefined] - def visit_test_result - visit(TestResult, test_result) + def visit_isolation_result + visit(IsolationResult, isolation_result) end # Original node diff --git a/lib/mutant/result.rb b/lib/mutant/result.rb index b3e449fd..997d9594 100644 --- a/lib/mutant/result.rb +++ b/lib/mutant/result.rb @@ -197,30 +197,30 @@ module Mutant # Mutation result class Mutation include Result, Anima.new( + :isolation_result, :mutation, - :test_result + :runtime ) - # The runtime + # Time the tests had been running # # @return [Float] - def runtime - test_result.runtime + def killtime + if isolation_result.success? + isolation_result.value.runtime + else + 0.0 + end end - # The time spent on killing - # - # @return [Float] - # - # @api private - alias_method :killtime, :runtime - # Test if mutation was handled successfully # # @return [Boolean] def success? - mutation.class.success?(test_result) + isolation_result.success? && + mutation.class.success?(isolation_result.value) end + memoize :success? end # Mutation end # Result diff --git a/spec/support/shared_context.rb b/spec/support/shared_context.rb index 9ff6d636..ba3b23d4 100644 --- a/spec/support/shared_context.rb +++ b/spec/support/shared_context.rb @@ -95,18 +95,24 @@ module SharedContext let(:mutation_a_result) do Mutant::Result::Mutation.new( - mutation: mutation_a, - test_result: mutation_a_test_result + mutation: mutation_a, + isolation_result: mutation_a_isolation_result, + runtime: 1.0 ) end let(:mutation_b_result) do Mutant::Result::Mutation.new( - mutation: mutation_a, - test_result: mutation_b_test_result + isolation_result: mutation_b_isolation_result, + mutation: mutation_b, + runtime: 1.0 ) end + let(:mutation_a_isolation_result) do + Mutant::Isolation::Result::Success.new(mutation_a_test_result) + end + let(:mutation_a_test_result) do Mutant::Result::Test.new( tests: [test_a], @@ -125,6 +131,10 @@ module SharedContext ) end + let(:mutation_b_isolation_result) do + Mutant::Isolation::Result::Success.new(mutation_b_test_result) + end + let(:subject_a_result) do Mutant::Result::Subject.new( subject: subject_a, diff --git a/spec/unit/mutant/env_spec.rb b/spec/unit/mutant/env_spec.rb index 54e7cd90..9460ceae 100644 --- a/spec/unit/mutant/env_spec.rb +++ b/spec/unit/mutant/env_spec.rb @@ -14,14 +14,14 @@ RSpec.describe Mutant::Env do ) end - let(:integration) { instance_double(Mutant::Integration) } - let(:test_a) { instance_double(Mutant::Test) } - let(:test_b) { instance_double(Mutant::Test) } - let(:tests) { [test_a, test_b] } - let(:selector) { instance_double(Mutant::Selector) } - let(:integration_class) { Mutant::Integration::Null } - let(:isolation) { instance_double(Mutant::Isolation::Fork) } - let(:mutation_subject) { instance_double(Mutant::Subject) } + let(:integration) { instance_double(Mutant::Integration) } + let(:test_a) { instance_double(Mutant::Test) } + let(:test_b) { instance_double(Mutant::Test) } + let(:tests) { [test_a, test_b] } + let(:selector) { instance_double(Mutant::Selector) } + let(:integration_class) { Mutant::Integration::Null } + let(:isolation) { Mutant::Isolation::None.new } + let(:mutation_subject) { instance_double(Mutant::Subject) } let(:mutation) do instance_double( @@ -39,7 +39,7 @@ RSpec.describe Mutant::Env do end before do - expect(selector).to receive(:call) + allow(selector).to receive(:call) .with(mutation_subject) .and_return(tests) @@ -53,8 +53,9 @@ RSpec.describe Mutant::Env do specify do should eql( Mutant::Result::Mutation.new( - mutation: mutation, - test_result: test_result + isolation_result: isolation_result, + mutation: mutation, + runtime: 1.0 ) ) end @@ -64,10 +65,6 @@ RSpec.describe Mutant::Env do let(:test_result) { instance_double(Mutant::Result::Test) } before do - expect(isolation).to receive(:call) - .ordered - .and_yield - expect(mutation).to receive(:insert) .ordered .with(config.kernel) @@ -78,22 +75,22 @@ RSpec.describe Mutant::Env do .and_return(test_result) end + let(:isolation_result) do + Mutant::Isolation::Result::Success.new(test_result) + end + include_examples 'mutation kill' end - context 'when isolation does raise error' do + context 'when code does raise error' do + let(:exception) { RuntimeError.new('foo') } + before do - expect(isolation).to receive(:call) - .and_raise(Mutant::Isolation::Error, 'test-error') + expect(mutation).to receive(:insert).and_raise(exception) end - let(:test_result) do - Mutant::Result::Test.new( - output: 'test-error', - passed: false, - runtime: 1.0, - tests: tests - ) + let(:isolation_result) do + Mutant::Isolation::Result::Error.new(exception) end include_examples 'mutation kill' diff --git a/spec/unit/mutant/isolation/fork_spec.rb b/spec/unit/mutant/isolation/fork_spec.rb index 6f5440df..2f669f74 100644 --- a/spec/unit/mutant/isolation/fork_spec.rb +++ b/spec/unit/mutant/isolation/fork_spec.rb @@ -128,12 +128,14 @@ RSpec.describe Mutant::Isolation::Fork do specify do XSpec::ExpectationVerifier.verify(self, expectations) do - expect(subject).to be(block_return) + expect(subject).to eql(Mutant::Isolation::Result::Success.new(block_return)) end end end context 'when fork fails' do + let(:exception) { RuntimeError.new('fork(2)') } + let(:expectations) do [ *prefork_expectations, @@ -141,7 +143,7 @@ RSpec.describe Mutant::Isolation::Fork do receiver: process, selector: :fork, reaction: { - exception: RuntimeError.new('fork(2)') + exception: exception } } ].map(&XSpec::MessageExpectation.method(:parse)) @@ -149,7 +151,7 @@ RSpec.describe Mutant::Isolation::Fork do specify do XSpec::ExpectationVerifier.verify(self, expectations) do - expect { expect(subject) }.to raise_error(described_class::Error, 'fork(2)') + expect(subject).to eql(Mutant::Isolation::Result::Error.new(exception)) end end end diff --git a/spec/unit/mutant/isolation/none_spec.rb b/spec/unit/mutant/isolation/none_spec.rb index bd3c3767..6bdf7874 100644 --- a/spec/unit/mutant/isolation/none_spec.rb +++ b/spec/unit/mutant/isolation/none_spec.rb @@ -4,15 +4,20 @@ RSpec.describe Mutant::Isolation::None do describe '.call' do let(:object) { described_class.new } - it 'return block value' do - expect(object.call { :foo }).to be(:foo) + context 'without exception' do + it 'returns success result' do + expect(object.call { :foo }) + .to eql(Mutant::Isolation::Result::Success.new(:foo)) + end end - it 'wraps *all* exceptions' do - expect { object.call { fail 'foo' } }.to raise_error( - Mutant::Isolation::Error, - 'foo' - ) + context 'with exception' do + let(:exception) { RuntimeError.new('foo') } + + it 'returns error result' do + expect(object.call { fail exception }) + .to eql(Mutant::Isolation::Result::Error.new(exception)) + end end end end diff --git a/spec/unit/mutant/isolation/result_spec.rb b/spec/unit/mutant/isolation/result_spec.rb new file mode 100644 index 00000000..2412416c --- /dev/null +++ b/spec/unit/mutant/isolation/result_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.describe Mutant::Isolation::Result do + describe '#success?' do + let(:value) { double('Object') } + + def apply + effective_class.new(value).success? + end + + context 'on success instance' do + let(:effective_class) { described_class::Success } + + it 'returns true' do + expect(apply).to be(true) + end + end + + context 'on error instance' do + let(:effective_class) { described_class::Error } + + it 'returns true' do + expect(apply).to be(false) + end + end + end +end diff --git a/spec/unit/mutant/reporter/cli/printer/isolation_result_spec.rb b/spec/unit/mutant/reporter/cli/printer/isolation_result_spec.rb new file mode 100644 index 00000000..d5bd588f --- /dev/null +++ b/spec/unit/mutant/reporter/cli/printer/isolation_result_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +RSpec.describe Mutant::Reporter::CLI::Printer::IsolationResult do + setup_shared_context + + describe '.call' do + context 'on failed isolation' do + let(:exception) do + Class.new(RuntimeError) do + def inspect + '' + end + + def backtrace + %w[first last] + end + end.new('foo') + end + + let(:reportable) do + Mutant::Isolation::Result::Error.new(exception) + end + + it_reports <<~'STR' + Killing the mutation resulted in an integration error. + This is the case when the tests selected for the current mutation + did not produce a test result, but instead an exception was raised. + + This may point to the following problems: + * Bug in mutant + * Bug in the ruby interpreter + * Bug in your test suite + * Bug in your test suite under concurrency + + The following exception was raised: + + ``` + + first + last + ``` + STR + end + + context 'on sucessful isolation' do + let(:reportable) do + Mutant::Isolation::Result::Success.new(mutation_a_test_result) + end + + it_reports <<~'STR' + - 1 @ runtime: 1.0 + - test-a + Test Output: + mutation a test result output + STR + end + end +end diff --git a/spec/unit/mutant/result/mutation_spec.rb b/spec/unit/mutant/result/mutation_spec.rb index ca64857e..b86cd2ac 100644 --- a/spec/unit/mutant/result/mutation_spec.rb +++ b/spec/unit/mutant/result/mutation_spec.rb @@ -3,8 +3,9 @@ RSpec.describe Mutant::Result::Mutation do let(:object) do described_class.new( - mutation: mutation, - test_result: test_result + isolation_result: isolation_result, + mutation: mutation, + runtime: 2.0 ) end @@ -17,23 +18,53 @@ RSpec.describe Mutant::Result::Mutation do ) end + let(:isolation_result) do + Mutant::Isolation::Result::Success.new(test_result) + end + + shared_examples_for 'unsuccessful isolation' do + let(:isolation_result) do + Mutant::Isolation::Result::Error.new(RuntimeError.new('foo')) + end + end + + describe '#killtime' do + subject { object.killtime } + + context 'if isolation is successful' do + it { should eql(1.0) } + end + + context 'if isolation is not successful' do + include_context 'unsuccessful isolation' + + it { should eql(0.0) } + end + end + describe '#runtime' do subject { object.runtime } - it { should eql(1.0) } + it { should eql(2.0) } end describe '#success?' do subject { object.success? } - let(:result) { double('result boolean') } + context 'if isolation is successful' do + before do + expect(mutation.class).to receive(:success?) + .with(test_result) + .and_return(true) + end - before do - expect(mutation.class).to receive(:success?) - .with(test_result) - .and_return(result) + it { should be(true) } end - it { should be(result) } + context 'if isolation is not successful' do + include_context 'unsuccessful isolation' + + it { should be(false) } + end end end