Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refute blank #707

Merged
merged 6 commits into from
Dec 25, 2016
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/extensions/object.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class Object
def blank?
return true if self.nil? || self.empty?
false
end
end
1 change: 1 addition & 0 deletions lib/faker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -233,5 +233,6 @@ def rand_in_range(from, to)

require 'extensions/array'
require 'extensions/symbol'
require 'extensions/object'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some frameworks (rails, for example) already define this extension and overwriting this could affect someone else's implementation of it causing subtle and hard to find bugs... possibly based on gem loading order or dev/prod environment... maybe just require this in the test_helper ? On the other hand, then if someone ended up using Object#blank? in the faker code somewhere, the tests could still pass but the gem wouldn't work except while running the tests, unless you had another gem defining this, and then it'd be based on someone else's implementation. Or maybe just check if it's already defined yet before redefining it. But I would question this in general because this could have effects beyond this gem. What functionality is this buying us for such a big risk? Using it in this way I think it at least needs stringent tests around it at a minimum.

Copy link
Contributor

@b264 b264 Sep 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about

class Tester < BasicObject
  def initialize object
    @object= object
  end
  def blank?
    (@object== nil) || (@object.respond_to?(:empty?) && @object.empty?)
  end
end

and

refute Tester.new(phrase).blank? in the test
Then I'd wonder why we would use a single refute


require 'helpers/char'
31 changes: 15 additions & 16 deletions test/test_faker_hacker_talk.rb
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
require File.expand_path(File.dirname(__FILE__) + '/test_helper.rb')

class TestFakerHacker < Test::Unit::TestCase
class TestFakerHacker < Test::Unit::TestCase
def setup
@hacker = Faker::Hacker
@phrases = @hacker.phrases
@phrases = @hacker.phrases
end
def test_phrases
assert @phrases.size == 8

def test_phrases
assert @phrases.size == 8
@phrases.each do |phrase|
assert !phrase.nil?
assert phrase != ""
end
end

def test_noun
refute phrase.blank?
end
end

def test_noun
assert @hacker.noun.match(/\w+/)
end

def test_abbreviation
assert @hacker.abbreviation.match(/\w+/)
end

def test_adjective
assert @hacker.adjective.match(/\w+/)
end

def test_verb
assert @hacker.verb.match(/\w+/)
end

def test_ingverb
assert @hacker.ingverb.match(/\w+/)
end

end
5 changes: 2 additions & 3 deletions test/test_faker_music.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@ def setup
def test_keys
assert @tester.keys.size == 7
@tester.keys.each do |key|
assert !key.nil?
assert key != ""
refute key.blank?
end
end

def test_key_variants
assert @tester.key_variants.size == 3
@tester.key_variants.each do |key_variant|
assert !key_variant.nil?
refute key_variant.nil?
end
end

Expand Down
13 changes: 6 additions & 7 deletions test/test_faker_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,27 @@ class TestFakerName < Test::Unit::TestCase
def setup
@tester = Faker::Name
end

def test_name
assert @tester.name.match(/(\w+\.? ?){2,3}/)
end

def test_name_with_middle
assert @tester.name_with_middle.match(/(\w+\.? ?){3,4}/)
end

def test_prefix
assert @tester.prefix.match(/[A-Z][a-z]+\.?/)
end

def test_suffix
assert @tester.suffix.match(/[A-Z][a-z]*\.?/)
end

def test_job_titles
@job_titles = Faker::Name.job_titles
@job_titles.each do |title|
assert !title.nil?
assert title != ""
end
refute title.blank?
end
end
end
58 changes: 27 additions & 31 deletions test/test_faker_shakespeare.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,49 @@
require File.expand_path(File.dirname(__FILE__) + '/test_helper.rb')

class TestFakerShakespeare < Test::Unit::TestCase
class TestFakerShakespeare < Test::Unit::TestCase
def setup
@romeo_and_juliet = Faker::Shakespeare.romeo_and_juliet
@king_richard_iii = Faker::Shakespeare.king_richard_iii
@as_you_like_it = Faker::Shakespeare.as_you_like_it
@hamlet = Faker::Shakespeare.hamlet
end

def test_quotes
assert @romeo_and_juliet.size == 11
@romeo_and_juliet.each do |quotes|
assert !quotes.nil?
assert quotes != ""
end

assert @king_richard_iii.size == 8
@king_richard_iii.each do |quotes|
assert !quotes.nil?
assert quotes != ""
end

assert @as_you_like_it.size == 8
@as_you_like_it.each do |quotes|
assert !quotes.nil?
assert quotes != ""
end

assert @hamlet.size == 18
@hamlet.each do |quotes|
assert !quotes.nil?
assert quotes != ""
end
end

assert @romeo_and_juliet.size == 11
@romeo_and_juliet.each do |quote|
refute quote.blank?
end

assert @king_richard_iii.size == 8
@king_richard_iii.each do |quote|
refute quote.blank?
end

assert @as_you_like_it.size == 8
@as_you_like_it.each do |quote|
refute quote.blank?
end

assert @hamlet.size == 18
@hamlet.each do |quote|
refute quote.blank?
end
end

def test_as_you_like_it_quote
assert Faker::Shakespeare.as_you_like_it_quote.match(/\w+/)
end

def test_king_richard_iii_quote
assert Faker::Shakespeare.king_richard_iii_quote.match(/\w+/)
end

def test_romeo_and_juliet_quote
assert Faker::Shakespeare.romeo_and_juliet_quote.match(/\w+/)
end

def test_hamlet_quote
assert Faker::Shakespeare.hamlet_quote.match(/\w+/)
end

end
18 changes: 6 additions & 12 deletions test/test_faker_star_wars.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,32 @@ def setup
def test_strings
assert @characters.size == 33
@characters.each do |character|
assert !character.nil?
assert character != ""
refute character.blank?
end

assert @droids.size == 21
@droids.each do |droid|
assert !droid.nil?
assert droid != ""
refute droid.blank?
end

assert @planets.size == 20
@planets.each do |planet|
assert !planet.nil?
assert planet != ""
refute planet.blank?
end

assert @quotes.size == 30
@quotes.each do |quote|
assert !quote.nil?
assert quote != ""
refute quote.blank?
end

assert @species.size == 9
@species.each do |specie|
assert !specie.nil?
assert specie != ""
refute specie.blank?
end

assert @vehicles.size == 21
@vehicles.each do |vehicle|
assert !vehicle.nil?
assert vehicle != ""
refute vehicle.blank?
end
end

Expand Down