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 missing tests reported by SimpleCov #1241

Merged
merged 8 commits into from
May 27, 2018
Merged

Add missing tests reported by SimpleCov #1241

merged 8 commits into from
May 27, 2018

Conversation

aamarill
Copy link

@aamarill aamarill commented May 22, 2018

This PR is for issue #1233 created by @vbrazo

Current Coverage

was: 98.86% | is: 99.18%

coverage 2018-05-22 at 1 40 57 pm

@aamarill aamarill changed the title Fix 1233 Add more tests May 22, 2018
@vbrazo vbrazo self-requested a review May 23, 2018 02:07
@@ -70,8 +68,6 @@ def random_complex_type
rb_hash
when :array
rb_array
else
rb_integer
end
Copy link
Member

Choose a reason for hiding this comment

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

By reading the code, I noticed that the code will never pass in the else statements, therefore I believe we should just delete the statements.

Copy link
Author

@aamarill aamarill May 24, 2018

Choose a reason for hiding this comment

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

Those else statements give a bit more flexibility to quickly edit the constants in the class. But I wasn't sure if that's worth not getting 100% test coverage. Thanks for clarifying that!

Copy link
Member

Choose a reason for hiding this comment

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

Well, the code is basically working with the values inside the SIMPLE_TYPES and COMPLEX_TYPES arrays and as we can see, integer isn't declared.

@vbrazo vbrazo changed the title Add more tests Add missing tests reported by SimpleCov May 26, 2018
@vbrazo vbrazo merged commit b92878c into faker-ruby:master May 27, 2018
@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
* Add some tests

* Add more tests

* Remove unnecessary else statements

* Add faker unique generator missing test

* Add more unique generator tests

* Update changelog.md
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