Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

RSpec 3 #155

Merged
merged 14 commits into from

4 participants

@Ptico

No description provided.

@Ptico Ptico commented on the diff
spec/unit/mutant/cli_new_spec.rb
@@ -13,9 +13,9 @@
shared_examples_for 'a cli parser' do
subject { cli.config }
- its(:strategy) { should eql(expected_strategy) }
- its(:reporter) { should eql(expected_reporter) }
- its(:matcher) { should eql(expected_matcher) }
+ it { expect(subject.strategy).to eql(expected_strategy) }
@Ptico
Ptico added a note

This is the first working solution for removed its.
subject.strategy.should doesn't work for unknown reasons. Sorry

@dkubb Collaborator
dkubb added a note

I believe the .should syntax is either removed from RSpec 3.0 or it is at least deprecated.

In preparation for this upcoming change, this RSpec suite has been configured to only work with expect(...). It's cleaner, simpler, and doesn't require monkey patching Object. See: https://github.com/mbj/mutant/blob/master/spec/spec_helper.rb#L50-L52

@Ptico
Ptico added a note

Such a shame! Had a late night debugging and was sure that mutant uses .should syntax for specs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mockdeep mockdeep commented on the diff
spec/unit/mutant/rspec/killer_spec.rb
@@ -42,7 +42,7 @@
context 'when run exits zero' do
let(:exit_status) { 0 }
- its(:killed?) { should be(false) }
+ it { expect(subject.killed?).to be(false) }

You could also say expect(subject).not_to be_killed

@dkubb Collaborator
dkubb added a note

I just wish there was an rspec configuration that forced the to be_* and not_to be_* tests to force true and false rather than truthy or falsey values.

When I design an API I like to be absolutely precise about what the return values are. I hate it when APIs are loose, and you can get truthy or falsey values in one context, and in another you get an integer or string (like Kernel#defined?) that you can interpret a whole other way -- add a second method if you want to extend the behaviour.

I like the test that is here now though, I think it's more precise.

Makes sense. I usually choose to be_* because I like the readability of it, but I agree, it would be nice to be able to be exact about it. Similarly, that's why I use eq true instead of be_true.

@dkubb Collaborator
dkubb added a note

@mockdeep I use be(true) which does an #equal? test under the hood. eq uses #==, which should probably have the same end result since I think TrueClass#== and FalseClass#== are inherited from Object, which I think uses #equal? too.

EDIT: I guess it's mostly a matter of preference, but I always liked using be when I mean to check for an exact match against the object.

@mbj Owner
mbj added a note

@dkubb My xspec will have it. Morpher predicates already distinguish between value equality and pointer identity equality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
mutant.gemspec
@@ -34,7 +34,7 @@ Gem::Specification.new do |gem|
gem.add_runtime_dependency('inflecto', '~> 0.0.2')
gem.add_runtime_dependency('anima', '~> 0.2.0')
gem.add_runtime_dependency('concord', '~> 0.1.4')
- gem.add_runtime_dependency('rspec', '~> 2.14.1')
@mbj Owner
mbj added a note

@mockdeep We should remove rspec as a runtime dependency of mutant. That was the whole idea of the split it seems I forgot it. Its okay as a devtime deb.

@Ptico
Ptico added a note

Done

@mbj Owner
mbj added a note

k

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/mutant/rspec/killer.rb
@@ -32,7 +32,7 @@ def run
return false
end
- reporter = RSpec::Core::Reporter.new
+ reporter = RSpec.configuration.reporter
@mbj Owner
mbj added a note

Are you sure this change has no side effects? The reason I used an "bare" reporter is to make sure I get "NULL" report behavior.

@Ptico
Ptico added a note

I think there is only one side effect: when someone configures different output for reporters, which is not that bad because this type of users knows what they do.
If you don't like this behavior, we can return to bare reporter, but will need to create separate RSpec2 and RSpec3 killers. What do you think?

@mbj Owner
mbj added a note

Does the reporter you use print anything on stdin or stdout when killing mutations?

I'd be totally okay with creating a helper method null_reporter that has a rspec2 and rspec3 specific branch inside. If there is only one difference between rspec2 and rspec3 we can branch on Rspec::Core::Version.

Edit: That helper method would be used to return the reporter used while killing.

If we run into more trouble I'd create an mutant-rspec2 and a mutant-rspec3 gem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mbj
Owner

@Ptico I'm very happy with this PR. We need to check for the reporter issue before I can merge it. I'll be part of the upcoming 0.4 release I wanna do this weekend.

@Ptico

Lets discuss this today in IRC

@Ptico

@mbj PR updated, please take a look

lib/mutant/rspec/killer.rb
@@ -90,6 +88,40 @@ def all_example_groups
strategy.example_groups
end
+ # Instantiate and memoize RSpec reporter
+ #
+ # @return [RSpec::Core::Reporter]
+ #
+ # @api private
+ #
+ def reporter
+ @reporter ||= rspec2? ? rspec_reporter.new : rspec_reporter.new(strategy.configuration)
@mbj Owner
mbj added a note

Can you replace this @ivar ||= expression with a query method and a memoize :reporter. I think instances of this class is frozen (or at least should).

@Ptico
Ptico added a note

I'm sorry, what do you mean by query method?

@dkubb Collaborator
dkubb added a note
@mbj Owner
mbj added a note

I thought about something like this

def reporter
  if rspec3?
    rspec2_reporter
  else
    rspec3_reporter
  end
end
memoize :reporter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/mutant/rspec/killer.rb
((20 lines not shown))
+ def rspec_reporter
+ RSpec::Core::Reporter
+ end
+
+ # Detect RSpec 2
+ #
+ # @return [true]
+ # when RSpec 2
+ #
+ # @return [false]
+ # otherwise
+ #
+ # @api private
+ #
+ def rspec2?
+ RSpec::Core::Version::STRING.split('.').first == '2'
@mbj Owner
mbj added a note

Can we use Version::STRING < '3.0.0' here? Should result in more clean code than split plus literal compare.

@mbj Owner
mbj added a note

Or even better: Version::STRING =~ /\A2\./?

@dkubb Collaborator
dkubb added a note

Yeah, you'd probably want to compare it's >= 2 also.

@mbj Owner
mbj added a note

Or even better better, Version::STRING.start_with('2.0') ?

@dkubb Collaborator
dkubb added a note

Good call. I'd use Version::STRING.start_with?('2.')

@Ptico
Ptico added a note

:+1:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mbj
Owner
mbj commented
@mbj mbj merged commit 58fd5fb into mbj:master

1 check was pending

Details default The Travis CI build is in progress
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
1  .gitignore
@@ -33,6 +33,7 @@ measurements
## BUNDLER
.bundle
Gemfile.lock
+Gemfile.*.lock
## PROJECT::SPECIFIC
/vendor
View
2  config/flay.yml
@@ -1,3 +1,3 @@
---
threshold: 18
-total_score: 811
+total_score: 808
View
1  config/reek.yml
@@ -136,4 +136,5 @@ UtilityFunction:
- Mutant::Rspec::Strategy#configuration
- Mutant::Rspec::Strategy#options
- Mutant::Rspec::Strategy#world
+ - Mutant::Rspec::Strategy#rspec2
max_helper_calls: 0
View
19 lib/mutant/rspec/killer.rb
@@ -32,8 +32,6 @@ def run
return false
end
- reporter = RSpec::Core::Reporter.new
-
example_groups.each do |group|
return true unless group.run(reporter)
end
@@ -90,6 +88,23 @@ def all_example_groups
strategy.example_groups
end
+ # Choose and memoize RSpec reporter
+ #
+ # @return [RSpec::Core::Reporter]
+ #
+ # @api private
+ #
+ def reporter
+ reporter_class = RSpec::Core::Reporter
+
+ if strategy.rspec2?
+ reporter_class.new
+ else
+ reporter_class.new(strategy.configuration)
+ end
+ end
+ memoize :reporter, freezer: :noop
+
end # Killer
end # Rspec
end # Mutant
View
16 lib/mutant/rspec/strategy.rb
@@ -46,6 +46,20 @@ def example_groups
world.example_groups
end
+ # Detect RSpec 2
+ #
+ # @return [true]
+ # when RSpec 2
+ #
+ # @return [false]
+ # otherwise
+ #
+ # @api private
+ #
+ def rspec2?
+ RSpec::Core::Version::STRING.start_with?('2.')
+ end
+
private
# Return world
@@ -67,7 +81,7 @@ def world
#
def options
options = RSpec::Core::ConfigurationOptions.new(%w(--fail-fast spec))
- options.parse_options
+ options.parse_options if rspec2?
options
end
memoize :options, freezer: :noop
View
2  mutant-rspec.gemspec
@@ -18,7 +18,7 @@ Gem::Specification.new do |gem|
gem.extra_rdoc_files = %w[TODO LICENSE]
gem.add_runtime_dependency('mutant', "~> #{gem.version}")
- gem.add_runtime_dependency('rspec-core', '~> 2.14.1')
+ gem.add_runtime_dependency('rspec-core', '>= 2.14.1', '<= 3.0.0.beta2')
gem.add_development_dependency('bundler', '~> 1.3', '>= 1.3.5')
end
View
62 spec/integration/mutant/rspec_spec.rb
@@ -4,35 +4,53 @@
describe Mutant, 'rspec integration' do
- around do |example|
- Dir.chdir(TestApp.root) do
- example.run
+ let(:base_cmd) { "bundle exec mutant -I lib --require test_app --use rspec" }
+
+ shared_examples_for 'rspec integration' do
+ around do |example|
+ Bundler.with_clean_env do
+ Dir.chdir(TestApp.root) do
+ Kernel.system("bundle install --gemfile=#{gemfile}")
+ ENV['BUNDLE_GEMFILE'] = gemfile
+ example.run
+ end
+ end
end
- end
- let(:base_cmd) { 'bundle exec mutant -I lib --require test_app --use rspec' }
+ specify 'it allows to kill mutations' do
+ expect(Kernel.system("#{base_cmd} ::TestApp::Literal#string")).to be(true)
+ end
- specify 'it allows to kill mutations' do
- expect(Kernel.system("#{base_cmd} ::TestApp::Literal#string")).to be(true)
- end
+ specify 'it allows to exclude mutations' do
+ cli = <<-CMD.split("\n").join(' ')
+ #{base_cmd}
+ ::TestApp::Literal#string
+ ::TestApp::Literal#uncovered_string
+ --ignore-subject ::TestApp::Literal#uncovered_string
+ CMD
+ expect(Kernel.system(cli)).to be(true)
+ end
- specify 'it allows to exclude mutations' do
- cli = <<-CMD.split("\n").join(' ')
- #{base_cmd}
- ::TestApp::Literal#string
- ::TestApp::Literal#uncovered_string
- --ignore-subject ::TestApp::Literal#uncovered_string
- CMD
- expect(Kernel.system(cli)).to be(true)
+ specify 'fails to kill mutations when they are not covered' do
+ cli = "#{base_cmd} ::TestApp::Literal#uncovered_string"
+ expect(Kernel.system(cli)).to be(false)
+ end
+
+ specify 'fails when some mutations are not covered' do
+ cli = "#{base_cmd} ::TestApp::Literal"
+ expect(Kernel.system(cli)).to be(false)
+ end
end
- specify 'fails to kill mutations when they are not covered' do
- cli = "#{base_cmd} ::TestApp::Literal#uncovered_string"
- expect(Kernel.system(cli)).to be(false)
+ context 'RSpec 2' do
+ let(:gemfile) { 'Gemfile.rspec2' }
+
+ it_behaves_like 'rspec integration'
end
- specify 'fails when some mutations are not covered' do
- cli = "#{base_cmd} ::TestApp::Literal"
- expect(Kernel.system(cli)).to be(false)
+ context 'Rspec 3' do
+ let(:gemfile) { 'Gemfile.rspec3' }
+
+ it_behaves_like 'rspec integration'
end
end
View
6 spec/unit/mutant/cli_new_spec.rb
@@ -13,9 +13,9 @@
shared_examples_for 'a cli parser' do
subject { cli.config }
- its(:strategy) { should eql(expected_strategy) }
- its(:reporter) { should eql(expected_reporter) }
- its(:matcher) { should eql(expected_matcher) }
+ it { expect(subject.strategy).to eql(expected_strategy) }
@Ptico
Ptico added a note

This is the first working solution for removed its.
subject.strategy.should doesn't work for unknown reasons. Sorry

@dkubb Collaborator
dkubb added a note

I believe the .should syntax is either removed from RSpec 3.0 or it is at least deprecated.

In preparation for this upcoming change, this RSpec suite has been configured to only work with expect(...). It's cleaner, simpler, and doesn't require monkey patching Object. See: https://github.com/mbj/mutant/blob/master/spec/spec_helper.rb#L50-L52

@Ptico
Ptico added a note

Such a shame! Had a late night debugging and was sure that mutant uses .should syntax for specs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ it { expect(subject.reporter).to eql(expected_reporter) }
+ it { expect(subject.matcher).to eql(expected_matcher) }
end
describe Mutant::CLI, '.new' do
View
4 spec/unit/mutant/rspec/killer_spec.rb
@@ -42,7 +42,7 @@
context 'when run exits zero' do
let(:exit_status) { 0 }
- its(:killed?) { should be(false) }
+ it { expect(subject.killed?).to be(false) }

You could also say expect(subject).not_to be_killed

@dkubb Collaborator
dkubb added a note

I just wish there was an rspec configuration that forced the to be_* and not_to be_* tests to force true and false rather than truthy or falsey values.

When I design an API I like to be absolutely precise about what the return values are. I hate it when APIs are loose, and you can get truthy or falsey values in one context, and in another you get an integer or string (like Kernel#defined?) that you can interpret a whole other way -- add a second method if you want to extend the behaviour.

I like the test that is here now though, I think it's more precise.

Makes sense. I usually choose to be_* because I like the readability of it, but I agree, it would be nice to be able to be exact about it. Similarly, that's why I use eq true instead of be_true.

@dkubb Collaborator
dkubb added a note

@mockdeep I use be(true) which does an #equal? test under the hood. eq uses #==, which should probably have the same end result since I think TrueClass#== and FalseClass#== are inherited from Object, which I think uses #equal? too.

EDIT: I guess it's mostly a matter of preference, but I always liked using be when I mean to check for an exact match against the object.

@mbj Owner
mbj added a note

@dkubb My xspec will have it. Morpher predicates already distinguish between value equality and pointer identity equality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
it { should be_a(described_class) }
end
@@ -50,7 +50,7 @@
context 'when run exits nonzero' do
let(:exit_status) { 1 }
- its(:killed?) { should be(true) }
+ it { expect(subject.killed?).to be(true) }
it { should be_a(described_class) }
end
View
4 test_app/Gemfile.rspec2
@@ -0,0 +1,4 @@
+source 'https://rubygems.org'
+
+gem 'rspec', '~> 2.14.1'
+gem 'mutant', path: '../'
View
3  test_app/Gemfile.rspec3
@@ -0,0 +1,3 @@
+source 'https://rubygems.org'
+gem 'rspec', '~> 3.0.0.beta2'
+gem 'mutant', path: '../'
Something went wrong with that request. Please try again.