Change isolation interface to separate isolation errors

* Before, when code inside the isolation failed outside the tests we
  counted this as a mutation kill.
* Now we report them explicitly as not killed mutations. As they where
  not killed by the tests.

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.

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'

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

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

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

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

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

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

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

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,

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'

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

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

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

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
+            '<TestException>'
+          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:
+
+        ```
+        <TestException>
+        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

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