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

Add some tests #1240

Merged
merged 1 commit into from
May 22, 2018
Merged

Add some tests #1240

merged 1 commit into from
May 22, 2018

Conversation

aamarill
Copy link

This PR is for issue #1233 created by @vbrazo

Side note:
I am not sure how to test the else cases of random_type and random_complex_type. I would love to hear any ideas you may have.

Current coverage

screen shot 2018-05-21 at 10 21 34 pm

Copy link
Member

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the first contribution 🥇

Our coverage was 98.53% and now it's 98.86% 💯

@@ -5,7 +5,7 @@ def setup
@tester = Faker::Measurement
end

def height
def test_height
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

I like the match and the expression that we pass. There is another way to validate the object type. You could do something like assert Faker::Name.first_name.is_a? String as well.

array = [1, 2, 3]
range = 1..10
assert array.include?(@tester.send(:resolve, array))
assert range.include?(@tester.send(:resolve, range))
Copy link
Member

Choose a reason for hiding this comment

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

👍

@vbrazo vbrazo merged commit 1380eb2 into faker-ruby:master May 22, 2018
@aamarill aamarill deleted the fix-1233 branch May 22, 2018 14:20
@aamarill aamarill restored the fix-1233 branch May 22, 2018 14:20
@vbrazo vbrazo self-requested a review July 19, 2018 01:36
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants