From 9ab35de2c4a331efc838e3400ba8df62b942a025 Mon Sep 17 00:00:00 2001 From: Markus Schirp Date: Sat, 21 Nov 2015 22:02:51 +0000 Subject: [PATCH] Reduce direct reference of globals --- bin/mutant | 6 +- config/flay.yml | 2 +- lib/mutant.rb | 6 +- lib/mutant/cli.rb | 29 +++++---- lib/mutant/config.rb | 4 ++ lib/mutant/env.rb | 2 +- lib/mutant/env/bootstrap.rb | 4 +- lib/mutant/loader.rb | 51 ++++++--------- lib/mutant/matcher/method.rb | 2 +- lib/mutant/mutation.rb | 11 +++- lib/mutant/repository.rb | 16 ++--- lib/mutant/runner.rb | 3 +- spec/unit/mutant/cli_spec.rb | 18 +++-- spec/unit/mutant/env_spec.rb | 65 ++++++++++++------- spec/unit/mutant/loader/eval_spec.rb | 44 ------------- spec/unit/mutant/loader_spec.rb | 42 ++++++++++++ .../mutant/matcher/method/instance_spec.rb | 1 + .../mutant/matcher/method/singleton_spec.rb | 1 + spec/unit/mutant/mutation_spec.rb | 16 +++-- spec/unit/mutant/repository/diff_spec.rb | 42 +++++++----- spec/unit/mutant/runner_spec.rb | 17 ++++- 21 files changed, 220 insertions(+), 162 deletions(-) delete mode 100644 spec/unit/mutant/loader/eval_spec.rb create mode 100644 spec/unit/mutant/loader_spec.rb diff --git a/bin/mutant b/bin/mutant index 00d784c7..63516c24 100755 --- a/bin/mutant +++ b/bin/mutant @@ -15,7 +15,11 @@ namespace = load_path: $LOAD_PATH, kernel: Kernel, pathname: Pathname, - require_highjack: Mutant::RequireHighjack.method(:call).to_proc.curry.call(Kernel), + require_highjack: Mutant::RequireHighjack + .method(:call) + .to_proc + .curry + .call(Kernel), root_require: 'mutant', includes: %w[ mutant diff --git a/config/flay.yml b/config/flay.yml index 9d14a4ef..fdafaff5 100644 --- a/config/flay.yml +++ b/config/flay.yml @@ -1,3 +1,3 @@ --- threshold: 18 -total_score: 1177 +total_score: 1173 diff --git a/lib/mutant.rb b/lib/mutant.rb index 46d8cc2f..ebbbd895 100644 --- a/lib/mutant.rb +++ b/lib/mutant.rb @@ -69,8 +69,8 @@ require 'mutant/parallel/worker' require 'mutant/parallel/source' require 'mutant/warning_filter' require 'mutant/require_highjack' -require 'mutant/mutator' require 'mutant/mutation' +require 'mutant/mutator' require 'mutant/mutator/registry' require 'mutant/mutator/util' require 'mutant/mutator/util/array' @@ -202,7 +202,11 @@ module Mutant integration: Integration::Null, isolation: Mutant::Isolation::Fork, jobs: ::Parallel.processor_count, + kernel: Kernel, + load_path: $LOAD_PATH, matcher: Matcher::Config::DEFAULT, + open3: Open3, + pathname: Pathname, requires: EMPTY_ARRAY, reporter: Reporter::CLI.build($stdout), zombie: false diff --git a/lib/mutant/cli.rb b/lib/mutant/cli.rb index 8f672783..dd9b7cae 100644 --- a/lib/mutant/cli.rb +++ b/lib/mutant/cli.rb @@ -1,26 +1,24 @@ module Mutant # Commandline parser + # + # rubocop:disable ClassLength class CLI include Adamantium::Flat, Equalizer.new(:config), Procto.call(:config) # Error failed when CLI argv is invalid Error = Class.new(RuntimeError) - EXIT_FAILURE = 1 - EXIT_SUCCESS = 0 - # Run cli with arguments # # @param [Array] arguments # - # @return [Fixnum] - # the exit status + # @return [Boolean] def self.run(arguments) - Runner.call(Env::Bootstrap.call(call(arguments))).success? ? EXIT_SUCCESS : EXIT_FAILURE + Runner.call(Env::Bootstrap.call(call(arguments))).success? rescue Error => exception $stderr.puts(exception.message) - EXIT_FAILURE + false end # Initialize object @@ -53,7 +51,7 @@ module Mutant def parse(arguments) opts = OptionParser.new do |builder| builder.banner = 'usage: mutant [options] MATCH_EXPRESSION ...' - %w[add_environment_options add_mutation_options add_filter_options add_debug_options].each do |name| + %i[add_environment_options add_mutation_options add_filter_options add_debug_options].each do |name| __send__(name, builder) end end @@ -138,7 +136,16 @@ module Mutant add_matcher(:ignore_expressions, config.expression_parser.(pattern)) end opts.on('--since REVISION', 'Only select subjects touched since REVISION') do |revision| - add_matcher(:subject_filters, Repository::SubjectFilter.new(Repository::Diff.from_head(revision))) + add_matcher( + :subject_filters, + Repository::SubjectFilter.new( + Repository::Diff.new( + config: config, + from: Repository::Diff::HEAD, + to: revision + ) + ) + ) end end @@ -153,14 +160,14 @@ module Mutant end opts.on('--version', 'Print mutants version') do puts("mutant-#{VERSION}") - Kernel.exit(EXIT_SUCCESS) + config.kernel.exit end opts.on('-d', '--debug', 'Enable debugging output') do with(debug: true) end opts.on_tail('-h', '--help', 'Show this message') do puts(opts.to_s) - Kernel.exit(EXIT_SUCCESS) + config.kernel.exit end end diff --git a/lib/mutant/config.rb b/lib/mutant/config.rb index 9aec35fe..cd641b7f 100644 --- a/lib/mutant/config.rb +++ b/lib/mutant/config.rb @@ -13,7 +13,11 @@ module Mutant :includes, :isolation, :jobs, + :kernel, + :load_path, :matcher, + :open3, + :pathname, :requires, :reporter, :zombie diff --git a/lib/mutant/env.rb b/lib/mutant/env.rb index c78a2cbc..06a0215a 100644 --- a/lib/mutant/env.rb +++ b/lib/mutant/env.rb @@ -44,7 +44,7 @@ module Mutant tests = selector.call(mutation.subject) config.isolation.call do - mutation.insert + mutation.insert(config.kernel) integration.call(tests) end rescue Isolation::Error => error diff --git a/lib/mutant/env/bootstrap.rb b/lib/mutant/env/bootstrap.rb index 243cad1c..03836587 100644 --- a/lib/mutant/env/bootstrap.rb +++ b/lib/mutant/env/bootstrap.rb @@ -92,8 +92,8 @@ module Mutant # # @return [undefined] def infect - config.includes.each(&$LOAD_PATH.method(:<<)) - config.requires.each(&Kernel.method(:require)) + config.includes.each(&config.load_path.method(:<<)) + config.requires.each(&config.kernel.method(:require)) @integration = config.integration.new(config).setup end diff --git a/lib/mutant/loader.rb b/lib/mutant/loader.rb index 30224a75..a83b75f2 100644 --- a/lib/mutant/loader.rb +++ b/lib/mutant/loader.rb @@ -1,38 +1,27 @@ module Mutant # Base class for code loaders class Loader - include AbstractType, Concord.new(:root, :subject), Procto.call + include Anima.new(:binding, :kernel, :node, :subject) - # Eval based loader - class Eval < self - - # Call loader - # - # @return [undefined] - # - # One off the very few valid uses of eval - # - # rubocop:disable Lint/Eval - def call - eval( - source, - TOPLEVEL_BINDING, - subject.source_path.to_s, - subject.source_line - ) - self - end - - private - - # Source generated from AST - # - # @return [String] - def source - Unparser.unparse(root) - end - - end # Eval + # Call loader + # + # @return [self] + def self.call(*arguments) + new(*arguments).call + end + # Call loader + # + # One off the very few valid uses of eval + # + # @return [undefined] + def call + kernel.eval( + Unparser.unparse(node), + binding, + subject.source_path.to_s, + subject.source_line + ) + end end # Loader end # Mutant diff --git a/lib/mutant/matcher/method.rb b/lib/mutant/matcher/method.rb index e048d8e7..bfe65eb7 100644 --- a/lib/mutant/matcher/method.rb +++ b/lib/mutant/matcher/method.rb @@ -85,7 +85,7 @@ module Mutant # # @return [Pathname] def source_path - Pathname.new(source_location.first) + env.config.pathname.new(source_location.first) end memoize :source_path diff --git a/lib/mutant/mutation.rb b/lib/mutant/mutation.rb index 9086f81b..d5a7af31 100644 --- a/lib/mutant/mutation.rb +++ b/lib/mutant/mutation.rb @@ -49,10 +49,17 @@ module Mutant # Insert mutated node # + # @param kernel [Kernel] + # # @return [self] - def insert + def insert(kernel) subject.prepare - Loader::Eval.call(root, subject) + Loader.call( + binding: TOPLEVEL_BINDING, + kernel: kernel, + node: root, + subject: subject + ) self end diff --git a/lib/mutant/repository.rb b/lib/mutant/repository.rb index b2c62de7..9aff1c9f 100644 --- a/lib/mutant/repository.rb +++ b/lib/mutant/repository.rb @@ -20,17 +20,9 @@ module Mutant # Diff between two objects in repository class Diff - include Adamantium, Concord.new(:from, :to) + include Adamantium, Anima.new(:config, :from, :to) HEAD = 'HEAD'.freeze - private_constant(*constants(false)) - - # Create diff from head to revision - # - # @return [Diff] - def self.from_head(to) - new(HEAD, to) - end # Test if diff changes file at line range # @@ -50,7 +42,7 @@ module Mutant -L #{line_range.begin},#{line_range.end}:#{path} ] - stdout, status = Open3.capture2(*command, binmode: true) + stdout, status = config.open3.capture2(*command, binmode: true) fail RepositoryError, "Command #{command} failed!" unless status.success? @@ -68,7 +60,7 @@ module Mutant # @return [Boolean] def tracks?(path) command = %W[git ls-files --error-unmatch -- #{path}] - Kernel.system( + config.kernel.system( *command, out: File::NULL, err: File::NULL @@ -81,7 +73,7 @@ module Mutant # # @return [TrueClass, nil] def within_working_directory?(path) - working_directory = Pathname.pwd + working_directory = config.pathname.pwd path.ascend { |parent| return true if working_directory.eql?(parent) } end diff --git a/lib/mutant/runner.rb b/lib/mutant/runner.rb index fbae380d..29f9188a 100644 --- a/lib/mutant/runner.rb +++ b/lib/mutant/runner.rb @@ -37,12 +37,13 @@ module Mutant # the last returned status payload def run_driver(driver) status = nil + sleep = env.config.kernel.method(:sleep) loop do status = driver.status reporter.progress(status) break if status.done - Kernel.sleep(reporter.delay) + sleep.call(reporter.delay) end driver.stop diff --git a/spec/unit/mutant/cli_spec.rb b/spec/unit/mutant/cli_spec.rb index 393d2df4..bdda4a7e 100644 --- a/spec/unit/mutant/cli_spec.rb +++ b/spec/unit/mutant/cli_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Mutant::CLI do let(:report_success) { true } it 'exits failure' do - expect(subject).to be(0) + expect(subject).to be(true) end end @@ -41,7 +41,7 @@ RSpec.describe Mutant::CLI do let(:report_success) { false } it 'exits failure' do - expect(subject).to be(1) + expect(subject).to be(false) end end @@ -55,7 +55,7 @@ RSpec.describe Mutant::CLI do it 'exits failure' do expect($stderr).to receive(:puts).with('test-error') - expect(subject).to be(1) + expect(subject).to be(false) end end end @@ -110,7 +110,7 @@ RSpec.describe Mutant::CLI do before do expect($stdout).to receive(:puts).with(expected_message) - expect(Kernel).to receive(:exit).with(0) + expect(Kernel).to receive(:exit) end it_should_behave_like 'a cli parser' @@ -172,7 +172,7 @@ Options: let(:flags) { %w[--version] } before do - expect(Kernel).to receive(:exit).with(0) + expect(Kernel).to receive(:exit) expect($stdout).to receive(:puts).with("mutant-#{Mutant::VERSION}") end @@ -237,7 +237,13 @@ Options: let(:expected_matcher_config) do default_matcher_config.with( subject_filters: [ - Mutant::Repository::SubjectFilter.new(Mutant::Repository::Diff.new('HEAD', 'master')) + Mutant::Repository::SubjectFilter.new( + Mutant::Repository::Diff.new( + config: Mutant::Config::DEFAULT, + from: 'HEAD', + to: 'master' + ) + ) ] ) end diff --git a/spec/unit/mutant/env_spec.rb b/spec/unit/mutant/env_spec.rb index 1faf971e..21525531 100644 --- a/spec/unit/mutant/env_spec.rb +++ b/spec/unit/mutant/env_spec.rb @@ -13,21 +13,33 @@ RSpec.describe Mutant::Env do ) end - let(:integration) { instance_double(Mutant::Integration) } + let(:integration) { instance_double(Mutant::Integration) } + let(:wrapped_node) { instance_double(Parser::AST::Node) } + let(:context) { instance_double(Mutant::Context) } + 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(:config) do - Mutant::Config::DEFAULT.with(isolation: isolation, integration: integration_class) + let(:isolation) do + instance_double(Mutant::Isolation::Fork.singleton_class) end - let(:isolation) { instance_double(Mutant::Isolation::Fork.singleton_class) } - let(:mutation) { Mutant::Mutation::Evil.new(mutation_subject, Mutant::AST::Nodes::N_NIL) } - let(:wrapped_node) { instance_double(Parser::AST::Node) } - let(:context) { instance_double(Mutant::Context) } - 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(:mutation) do + instance_double( + Mutant::Mutation, + subject: mutation_subject + ) + end + + let(:config) do + Mutant::Config::DEFAULT.with( + isolation: isolation, + integration: integration_class, + kernel: instance_double(Kernel.singleton_class) + ) + end let(:mutation_subject) do instance_double( @@ -41,7 +53,14 @@ RSpec.describe Mutant::Env do subject { object.kill(mutation) } shared_examples_for 'mutation kill' do - it { should eql(Mutant::Result::Mutation.new(mutation: mutation, test_result: test_result)) } + specify do + should eql( + Mutant::Result::Mutation.new( + mutation: mutation, + test_result: test_result + ) + ) + end end before do @@ -53,25 +72,21 @@ RSpec.describe Mutant::Env do end context 'when isolation does not raise error' do - let(:test_result) { instance_double(Mutant::Result::Test, passed: false) } + let(:test_result) do + instance_double( + Mutant::Result::Test, + passed: false + ) + end before do expect(isolation).to receive(:call) .ordered .and_yield - expect(mutation_subject).to receive(:prepare) + expect(mutation).to receive(:insert) .ordered - .and_return(mutation_subject) - - expect(context).to receive(:root) - .with(s(:nil)) - .and_return(wrapped_node) - - expect(Mutant::Loader::Eval).to receive(:call) - .ordered - .with(wrapped_node, mutation_subject) - .and_return(Mutant::Loader::Eval) + .with(config.kernel) expect(integration).to receive(:call) .ordered diff --git a/spec/unit/mutant/loader/eval_spec.rb b/spec/unit/mutant/loader/eval_spec.rb deleted file mode 100644 index adb04e5b..00000000 --- a/spec/unit/mutant/loader/eval_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -RSpec.describe Mutant::Loader::Eval, '.call' do - - subject { object.call(node, mutation_subject) } - - let(:object) { Class.new(described_class) } - let(:path) { __FILE__ } - let(:line) { 1 } - - let(:mutation_subject) do - instance_double(Mutant::Subject, source_path: path, source_line: line) - end - - let(:source) do - <<-RUBY - class SomeNamespace - class Bar - def some_method - end - end - - class SomeOther - class Foo < Bar - end - end - end - RUBY - end - - let(:node) do - parse(source) - end - - it 'should load nodes into vm' do - subject - ::SomeNamespace::SomeOther::Foo - end - - it 'should set file and line correctly' do - subject - expect(::SomeNamespace::Bar - .instance_method(:some_method) - .source_location).to eql([__FILE__, 3]) - end -end diff --git a/spec/unit/mutant/loader_spec.rb b/spec/unit/mutant/loader_spec.rb new file mode 100644 index 00000000..1d550553 --- /dev/null +++ b/spec/unit/mutant/loader_spec.rb @@ -0,0 +1,42 @@ +RSpec.describe Mutant::Loader, '.call' do + subject do + described_class.call( + binding: binding, + kernel: kernel, + node: node, + subject: mutation_subject + ) + end + + let(:path) { instance_double(Pathname, to_s: path_str) } + let(:path_str) { instance_double(String) } + let(:line) { instance_double(Fixnum) } + let(:kernel) { instance_double(Kernel.singleton_class) } + let(:binding) { instance_double(Binding) } + let(:source) { instance_double(String) } + let(:node) { instance_double(Parser::AST::Node) } + + let(:mutation_subject) do + instance_double( + Mutant::Subject, + source_path: path, + source_line: line + ) + end + + it 'performs expected kernel interaction' do + expect(Unparser).to receive(:unparse) + .with(node) + .and_return(source) + + expect(kernel).to receive(:eval) + .with( + source, + binding, + path_str, + line + ) + + subject + end +end diff --git a/spec/unit/mutant/matcher/method/instance_spec.rb b/spec/unit/mutant/matcher/method/instance_spec.rb index 4ea5b54b..ff1c2b07 100644 --- a/spec/unit/mutant/matcher/method/instance_spec.rb +++ b/spec/unit/mutant/matcher/method/instance_spec.rb @@ -13,6 +13,7 @@ RSpec.describe Mutant::Matcher::Method::Instance, '#call' do let(:env) do instance_double( Mutant::Env::Bootstrap, + config: Mutant::Config::DEFAULT, parser: Fixtures::TEST_ENV.parser ) end diff --git a/spec/unit/mutant/matcher/method/singleton_spec.rb b/spec/unit/mutant/matcher/method/singleton_spec.rb index 139627d6..943cff34 100644 --- a/spec/unit/mutant/matcher/method/singleton_spec.rb +++ b/spec/unit/mutant/matcher/method/singleton_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Mutant::Matcher::Method::Singleton, '#call' do let(:env) do instance_double( Mutant::Env::Bootstrap, + config: Mutant::Config::DEFAULT, parser: Fixtures::TEST_ENV.parser ) end diff --git a/spec/unit/mutant/mutation_spec.rb b/spec/unit/mutant/mutation_spec.rb index a5cc6164..d51eaa4b 100644 --- a/spec/unit/mutant/mutation_spec.rb +++ b/spec/unit/mutant/mutation_spec.rb @@ -19,9 +19,10 @@ RSpec.describe Mutant::Mutation do let(:test_b) { instance_double(Mutant::Test) } describe '#insert' do - subject { object.insert } + subject { object.insert(kernel) } - let(:root_node) { s(:foo) } + let(:root_node) { s(:foo) } + let(:kernel) { instance_double(Kernel) } before do expect(context).to receive(:root) @@ -32,10 +33,15 @@ RSpec.describe Mutant::Mutation do .ordered .and_return(mutation_subject) - expect(Mutant::Loader::Eval).to receive(:call) + expect(Mutant::Loader).to receive(:call) .ordered - .with(root_node, mutation_subject) - .and_return(Mutant::Loader::Eval) + .with( + binding: TOPLEVEL_BINDING, + kernel: kernel, + node: root_node, + subject: mutation_subject + ) + .and_return(Mutant::Loader) end it_should_behave_like 'a command method' diff --git a/spec/unit/mutant/repository/diff_spec.rb b/spec/unit/mutant/repository/diff_spec.rb index 0cc04c9d..f821078f 100644 --- a/spec/unit/mutant/repository/diff_spec.rb +++ b/spec/unit/mutant/repository/diff_spec.rb @@ -1,22 +1,34 @@ describe Mutant::Repository::Diff do - describe '.from_head' do - subject { described_class.from_head(to_revision) } - - let(:to_revision) { instance_double(String) } - - it { should eql(described_class.new('HEAD', to_revision)) } - end - describe '#touches?' do - let(:object) { described_class.new('from_rev', 'to_rev') } - let(:path) { Pathname.pwd.join('foo.rb') } - let(:line_range) { 1..2 } + let(:object) do + described_class.new( + config: config, + from: 'from_rev', + to: 'to_rev' + ) + end + + let(:config) do + instance_double( + Mutant::Config, + kernel: kernel, + open3: open3, + pathname: pathname + ) + end + + let(:pathname) { instance_double(Pathname.singleton_class, pwd: pwd) } + let(:open3) { instance_double(Open3.singleton_class) } + let(:kernel) { instance_double(Kernel.singleton_class) } + let(:pwd) { Pathname.new('/foo') } + let(:path) { Pathname.new('/foo/bar.rb') } + let(:line_range) { 1..2 } subject { object.touches?(path, line_range) } shared_context 'test if git tracks the file' do before do - expect(Kernel).to receive(:system) + expect(config.kernel).to receive(:system) .ordered .with( *%W[git ls-files --error-unmatch -- #{path}], @@ -27,10 +39,10 @@ describe Mutant::Repository::Diff do end context 'when file is in a different subdirectory' do - let(:path) { Pathname.new('/foo.rb') } + let(:path) { Pathname.new('/baz/bar.rb') } before do - expect(Kernel).to_not receive(:system) + expect(config.kernel).to_not receive(:system) end it { should be(false) } @@ -53,7 +65,7 @@ describe Mutant::Repository::Diff do include_context 'test if git tracks the file' before do - expect(Open3).to receive(:capture2) + expect(config.open3).to receive(:capture2) .ordered .with(*expected_git_log_command, binmode: true) .and_return([stdout, status]) diff --git a/spec/unit/mutant/runner_spec.rb b/spec/unit/mutant/runner_spec.rb index f1bdb334..bd9a584f 100644 --- a/spec/unit/mutant/runner_spec.rb +++ b/spec/unit/mutant/runner_spec.rb @@ -4,22 +4,33 @@ RSpec.describe Mutant::Runner do let(:reporter) { instance_double(Mutant::Reporter, delay: delay) } let(:driver) { instance_double(Mutant::Parallel::Driver) } let(:delay) { instance_double(Float) } - let(:env) { instance_double(Mutant::Env, mutations: []) } let(:env_result) { instance_double(Mutant::Result::Env) } let(:actor_env) { instance_double(Mutant::Actor::Env) } + let(:kernel) { instance_double(Kernel.singleton_class) } + let(:sleep) { instance_double(Method) } + + let(:env) do + instance_double( + Mutant::Env, + actor_env: actor_env, + config: config, + mutations: [] + ) + end let(:config) do instance_double( Mutant::Config, integration: integration, jobs: 1, + kernel: kernel, reporter: reporter ) end before do - allow(env).to receive_messages(config: config, actor_env: actor_env) allow(env).to receive(:method).with(:kill).and_return(parallel_config.processor) + allow(kernel).to receive(:method).with(:sleep).and_return(sleep) end let(:parallel_config) do @@ -57,7 +68,7 @@ RSpec.describe Mutant::Runner do before do expect(driver).to receive(:status).and_return(status_a).ordered expect(reporter).to receive(:progress).with(status_a).ordered - expect(Kernel).to receive(:sleep).with(reporter.delay).ordered + expect(sleep).to receive(:call).with(reporter.delay).ordered expect(driver).to receive(:status).and_return(status_b).ordered expect(reporter).to receive(:progress).with(status_b).ordered