Skip to content

Commit

Permalink
Upgrade to RSpec 3
Browse files Browse the repository at this point in the history
* `should` syntax has been deprecated in favor of the newer `expect` syntax. The
specs were updated to use `expect`.
* `treat_symbols_as_metadata_keys_with_true_values` is now the default behavior in
RSpec 3 and is no longer a configuration option.
* Unused `factories.rb` file was removed
  • Loading branch information
JoelQ committed Jul 26, 2014
1 parent d6593be commit b3888d0
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 82 deletions.
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -6,6 +6,6 @@ gem 'colored'
gem 'rake'

group :test do
gem 'rspec'
gem 'rspec', '~> 3.0'
gem 'coveralls', require: false
end
10 changes: 0 additions & 10 deletions spec/factory.rb

This file was deleted.

76 changes: 39 additions & 37 deletions spec/rubycards/card_spec.rb
Expand Up @@ -6,28 +6,29 @@
describe '#initialize' do
context 'no params' do
it 'should return the ace of spades' do
subject.suit.should == 'Spades'
subject.rank.should == 'Ace'
new_card = Card.new
expect(new_card.suit).to eq 'Spades'
expect(new_card.rank).to eq 'Ace'
end
end

context 'params' do
it 'should return the card specified in params' do
new_card = Card.new('Queen', 'Clubs')
new_card.rank.should == 'Queen'
new_card.suit.should == 'Clubs'
expect(new_card.rank).to eq 'Queen'
expect(new_card.suit).to eq 'Clubs'

new_card = Card.new(3, 'Spades')
new_card.rank.should == 3.to_s
new_card.suit.should == 'Spades'
expect(new_card.rank).to eq '3'
expect(new_card.suit).to eq 'Spades'

new_card = Card.new('Jack', 'Diamonds')
new_card.rank.should == 'Jack'
new_card.suit.should == 'Diamonds'
expect(new_card.rank).to eq 'Jack'
expect(new_card.suit).to eq 'Diamonds'

new_card = Card.new(7, 'Hearts')
new_card.rank.should == 7.to_s
new_card.suit.should == 'Hearts'
expect(new_card.rank).to eq '7'
expect(new_card.suit).to eq 'Hearts'
end
end
end
Expand All @@ -42,20 +43,20 @@
let (:c4) { Card.new(4,'Spades') }

it 'should reflect the correct rank when compared' do
king.should > queen
king.should > jack
king.should == king
king.should < ace
expect(king).to be > queen
expect(king).to be > jack
expect(king).to eq king
expect(king).to be < ace

This comment has been minimized.

Copy link
@darrencauthon

darrencauthon Jul 26, 2014

Collaborator

This is the type of code that makes me question if a move to RSpec 3 is an "upgrade."

Look at this:

king.should > queen
king.should == king

versus

expect(king).to be > queen
expect(king).to be eq king

The latter is much more code, and awkward code as we have to wrap the subject and the value in function calls. But there's something darker in here, and it's in code in this PR like this:

# ...
expect(suits.count).to be 4
# ...
expect(runt_card1.rank).to be_nil
# ...
expect(deck1[index]).to be_same_as deck2[index]

Is it supposed to be .to eq 4 or .to be 4? Or should it be .to be object or .to be_same_as? Or is it .to be_nil or .to be nil? The answers to these are subjective, or it's "insider" stuff... you just have to know to use .to be_nil or whatever...

I'm not against this PR, but part of me wants to "upgrade" the tests by switching to Minitest... tests in that tend to be more simple and consistent.

Stopping my rant now.

This comment has been minimized.

Copy link
@jdan

jdan Jul 26, 2014

Owner

I'm all for a switch to Minitest

This comment has been minimized.

Copy link
@JoelQ

JoelQ Jul 26, 2014

Author Contributor

Agreed that some of the syntax surrounding be can be confusing. It actually follows a few simple rules:

  • be_ matchers are generated from ? methods on an object. So .to be_same_as check that .same_as? == true. It raises an error if the expected method is not defined on the object.
expect(Card.new).to be_awesome
# => expected #<Card:0x007f84999ca700> to respond to `awesome?`
  • .to be object checks object identity
  • .to eq object check object equality
  • </> operators are used with be, .to be > 5

Personally, I like that the new expect syntax wraps the subject and value in a function call instead of monkey-patching Object like the should syntax does. It only adds a few characters and in my personal experience, it doesn't impact test readability. MiniTest takes a similar approach of wrapping the subject and value in a function call with matchers such as assert_equal(subject, value).

This comment has been minimized.

Copy link
@joeletizia

joeletizia Jul 26, 2014

Collaborator

All of your reservations are the same I had the first time I upgraded a project from rspec2 to rspec3. Generally, I use the following rule.

expect(x).to eq(4) # testing for equality via the == operator
expect(x).to be(4) # testing against object identity

Generally, I use eq for value equality, as I would in the example below.

expect(suits.count).to be 4

For references to objects, which I'm generally going to stub, I use reference

expect(object.foo).to be(mock)

Of course, everything in ruby is an object...so I follow the pattern of "Always, sometimes, never."

I always stub external dependencies, and return a mock.
I sometimes (definitely more often than not) stub internal dependencies within my own project, and return a mock.
I never stub value type-ish return values (integers, strings) and I test against equality.

I've never used mini test, though.


jack.should_not > queen
jack.should_not > ace
jack.should < queen
expect(jack).not_to be > queen
expect(jack).not_to be > ace
expect(jack).to be < queen

ace.should_not < queen
ace.should > c4
expect(ace).not_to be < queen
expect(ace).to be > c4

c2.should < c4
c2_heart.should == c2
expect(c2).to be < c4
expect(c2_heart).to eq c2
end
end

Expand All @@ -64,19 +65,19 @@
let (:king) { Card.new('King', 'Clubs') }

it 'should return a long rank' do
king.rank.should == 'King'
expect(king.rank).to eq 'King'
end

it 'should return a short rank' do
king.rank(true).should == 'K'
expect(king.rank(true)).to eq 'K'
end
end

context 'numeric cards' do
let (:num) { Card.new(10, 'Diamonds') }

it 'should have the same long and short rank' do
num.rank.should == num.rank(true)
expect(num.rank).to eq num.rank(true)
end
end
end
Expand All @@ -85,14 +86,15 @@
BORDER_COUNT = 18 # the number of unicode characters on the border
RANKS = [*2..10, 'Jack', 'Queen', 'King', 'Ace']

it 'should have the correct number of glyps' do
it 'should have the correct number of glyphs' do
RANKS.each do |rank|
card = Card.new(rank, 'hearts') # rspec doesn't play nice with dark cards
glyph_count = card.to_s.scan(/[^\x00-\x7F]/).count

if (2..10).include? card.rank.to_i
card.to_s.scan(/[^\x00-\x7F]/).count.should == card.rank.to_i + BORDER_COUNT
expect(glyph_count).to eq card.rank.to_i + BORDER_COUNT
else
card.to_s.scan(/[^\x00-\x7F]/).count.should == 2 + BORDER_COUNT
expect(glyph_count).to eq 2 + BORDER_COUNT
end
end
end
Expand All @@ -101,7 +103,7 @@
RANKS.each do |rank|
card = Card.new(rank, 'diamonds')

card.inspect.should == card.short
expect(card.inspect).to eq card.short
end
end
end
Expand All @@ -112,17 +114,17 @@
runt_card2 = Card.new(11, 'Diamonds')
runt_card3 = Card.new(-6, 'Spades')

runt_card1.rank.should be_nil
runt_card2.rank.should be_nil
runt_card3.rank.should be_nil
expect(runt_card1.rank).to be_nil
expect(runt_card2.rank).to be_nil
expect(runt_card3.rank).to be_nil
end

it 'should set a garbage suit to nil' do
runt_card1 = Card.new(7, '')
runt_card2 = Card.new('Ace', 'Garbage')

runt_card1.suit.should be_nil
runt_card2.suit.should be_nil
expect(runt_card1.suit).to be_nil
expect(runt_card2.suit).to be_nil
end
end

Expand All @@ -131,17 +133,17 @@
deck1 = Deck.new
deck2 = Deck.new
deck1.cards.each_with_index do |_, index|
deck1[index].same_as?(deck2[index]).should == true
deck2[index].same_as?(deck1[index]).should == true
expect(deck1[index]).to be_same_as deck2[index]
expect(deck2[index]).to be_same_as deck1[index]
end
end

it "should return false when compared to different cards" do
deck1 = Deck.new
deck2 = Deck.new
deck1.cards.each_with_index do |_, index|
deck1[index].same_as?(deck2[index-1]).should == false
deck2[index].same_as?(deck1[index-1]).should == false
expect(deck1[index]).not_to be_same_as deck2[index-1]
expect(deck2[index]).not_to be_same_as deck1[index-1]
end
end
end
Expand Down
46 changes: 23 additions & 23 deletions spec/rubycards/deck_spec.rb
Expand Up @@ -7,27 +7,27 @@

describe '#initialize' do
it 'initializes 52 cards' do
deck.cards.count.should == 52
expect(deck.cards.count).to eq 52
# test indexing
deck[0].should be_a Card
expect(deck[0]).to be_a Card
# test enumerability
deck.each { |card| card.should be_a Card }
deck.each { |card| expect(card).to be_a Card }
end

it "initializes four suits" do
suits = deck.cards.group_by { |c| c.suit }
suits.count.should == 4
expect(suits.count).to be 4
end

["Clubs", "Diamonds", "Hearts", "Spades"].each do |suit|
it "initializes 13 cards for #{suit}" do
cards = deck.cards.select { |c| c.suit == suit }
cards.count.should == 13
expect(cards.count).to be 13
end

it "should include ranks of different ranks" do
cards = deck.cards.select { |c| c.suit == suit }
cards.group_by { |x| x.rank }.count.should == 13
expect(cards.group_by { |x| x.rank }.count).to be 13
end
end
end
Expand All @@ -36,26 +36,25 @@
it 'shuffles the cards' do
cards_before_shuffling = deck.cards.dup
deck.shuffle!
deck.cards.should_not == cards_before_shuffling
expect(deck.cards).not_to eq cards_before_shuffling
end

it "should should shuffle the internal cards array" do
cards_before_shuffling = deck.cards.dup
expect(deck.instance_variable_get("@cards")).to receive(:shuffle!)
deck.shuffle!
end

it 'returns itself' do
should == deck.shuffle!
expect(deck).to eq deck.shuffle!
end
end

describe '#draw' do
it 'draws a single card from the deck' do
first_card = deck.cards.first
cards_expected_after_draw = deck.cards[1..-1]
deck.draw.same_as?(first_card).should == true
deck.cards.should == cards_expected_after_draw
expect(deck.draw).to be_same_as first_card
expect(deck.cards).to eq cards_expected_after_draw
end
end

Expand All @@ -78,6 +77,7 @@
context 'cutting the first card' do

it 'returns itself' do
expect(deck).to eq deck.cut!(0)
should == deck.cut!(0)
end

Expand All @@ -86,19 +86,19 @@

deck.cut!(0)

deck.cards[0].suit.should == cards_before_cutting[1].suit
deck.cards[0].rank.should == cards_before_cutting[1].rank
expect(deck.cards[0].suit).to eq cards_before_cutting[1].suit
expect(deck.cards[0].rank).to eq cards_before_cutting[1].rank

deck.cards[-1].suit.should == cards_before_cutting[0].suit
deck.cards[-1].rank.should == cards_before_cutting[0].rank
expect(deck.cards[-1].suit).to eq cards_before_cutting[0].suit
expect(deck.cards[-1].rank).to eq cards_before_cutting[0].rank
end

it 'should return the same number of cards' do
cards_before_cutting = deck.cards.dup

deck.cut!(0)

deck.cards.count.should == cards_before_cutting.count
expect(deck.cards.count).to eq cards_before_cutting.count
end
end

Expand All @@ -108,21 +108,21 @@

deck.cut!(1)

deck.cards[0].suit.should == cards_before_cutting[2].suit
deck.cards[0].rank.should == cards_before_cutting[2].rank
expect(deck.cards[0].suit).to eq cards_before_cutting[2].suit
expect(deck.cards[0].rank).to eq cards_before_cutting[2].rank

deck.cards[-2].suit.should == cards_before_cutting[0].suit
deck.cards[-2].rank.should == cards_before_cutting[0].rank
deck.cards[-1].suit.should == cards_before_cutting[1].suit
deck.cards[-1].rank.should == cards_before_cutting[1].rank
expect(deck.cards[-2].suit).to eq cards_before_cutting[0].suit
expect(deck.cards[-2].rank).to eq cards_before_cutting[0].rank
expect(deck.cards[-1].suit).to eq cards_before_cutting[1].suit
expect(deck.cards[-1].rank).to eq cards_before_cutting[1].rank
end

it 'should return the same number of cards' do
cards_before_cutting = deck.cards.dup

deck.cut!(1)

deck.cards.count.should == cards_before_cutting.count
expect(deck.cards.count).to eq cards_before_cutting.count
end
end
end
Expand Down
20 changes: 10 additions & 10 deletions spec/rubycards/string_spec.rb
Expand Up @@ -7,42 +7,42 @@
context 'one liners' do
# cat <- dog == catdog
it 'should concatenate strings' do
'cat'.next('dog').should == 'catdog'
expect('cat'.next('dog')).to eq 'catdog'
end

# hello <- "" == hello
it 'should remain unchanged' do
'hello'.next('').should == 'hello'
expect('hello'.next('')).to eq 'hello'
end

# "" <- hello == ""
it 'should return an empty string' do
''.next('hello').should == ''
expect(''.next('hello')).to eq ''
end
end

context 'multiple lines' do
# aa bb aabb
# aa <- bb == aabb
it 'should place blocks adjacently' do
"aa\naa".next("bb\nbb").should == "aabb\naabb"
"aaa\naaa\naaa".next("bbb\nbbb\nbbb").should == "aaabbb\naaabbb\naaabbb"
expect("aa\naa".next("bb\nbb")).to eq "aabb\naabb"
expect("aaa\naaa\naaa".next("bbb\nbbb\nbbb")).to eq "aaabbb\naaabbb\naaabbb"
end

# aa bb aabb
# aa <- == aa
it 'should stack the second block in its entirety' do
"aa\naa".next("bb").should == "aabb\naa"
"1\n2\n3".next("4\n5").should == "14\n25\n3"
expect("aa\naa".next("bb")).to eq "aabb\naa"
expect("1\n2\n3".next("4\n5")).to eq "14\n25\n3"
end

# aa bb aabb
# <- bb ==
it 'should cut off some of the second block' do
"aa".next("bb\nbb").should == "aabb"
"1\n2".next("4\n5\n6").should == "14\n25"
expect("aa".next("bb\nbb")).to eq "aabb"
expect("1\n2".next("4\n5\n6")).to eq "14\n25"
end
end
end

end
end
1 change: 0 additions & 1 deletion spec/spec_helper.rb
Expand Up @@ -12,7 +12,6 @@

# See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration
RSpec.configure do |config|
config.treat_symbols_as_metadata_keys_with_true_values = true
config.run_all_when_everything_filtered = true
config.filter_run :focus

Expand Down

0 comments on commit b3888d0

Please sign in to comment.