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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: decimal with r_digits 1 always finishes in 0 #2048

Merged

Conversation

martinjaimem
Copy link
Contributor

@martinjaimem martinjaimem commented Jun 10, 2020

Resolves #2045

Description:

Right now there is a bug when generating a random decimal with r_digits: 1 as it always ends in 0:

100.times.map{Faker::Number.decimal(l_digits: 1, r_digits: 1)}
=> [3.0, 4.0, 1.0, 9.0, 5.0, 9.0, 1.0, 1.0, 2.0, 1.0, 7.0, 9.0, 4.0, 0.0, 0.0, 7.0, 6.0, 0.0, 2.0, 9.0, 5.0, 9.0, 9.0, 3.0, 2.0, 8.0, 9.0, 3.0, 2.0, 8.0, 8.0, 0.0, 8.0, 4.0, 9.0, 7.0, 7.0, 1.0, 0.0, 7.0, 6.0, 0.0, 6.0, 7.0, 9.0, 5.0, 4.0, 2.0, 3.0, 3.0, 8.0, 2.0, 8.0, 5.0, 9.0, 5.0, 9.0, 3.0, 0.0, 3.0, 4.0, 6.0, 1.0, 4.0, 6.0, 3.0, 5.0, 7.0, 4.0, 0.0, 6.0, 9.0, 4.0, 1.0, 9.0, 2.0, 9.0, 3.0, 6.0, 2.0, 1.0, 5.0, 4.0, 9.0, 3.0, 9.0, 8.0, 4.0, 1.0, 5.0, 8.0, 1.0, 0.0, 8.0, 0.0, 5.0, 9.0, 1.0, 2.0, 7.0]

This fixes it, plus there is a small change in the behavior (I really don't know if it was intended or not) that when generating a 1 right digit, it was not checking that it was not 0. After that the comment was:

# Ensure the last digit is not zero
# so it does not get truncated on converting to float

So now it is being checked that the last digit is not 0 even if it is only one digit.

Feel free to suggest any changes, improvements or to ask for more tests 馃槃

@martinjaimem
Copy link
Contributor Author

martinjaimem commented Jun 10, 2020

@Zeragamba @Newman101 Hi! Could you help me with sth?

I am facing a test that seems to be failing randomly. The test is:
faker/test/faker/music/test_faker_rush.rb

I think it has nothing to do with this PR as I only changed the decimal method that shouln't be related to music.

Anyway, the error is: NoMethodError: undefined method 'match' for 2112:Integer

It comes from the fetch method in the Faker::Base, when it does:

sample(translate("faker.#{key}"))

where translate("faker.#{key}") returns:

["Rush", "Fly by Night", "Caress of Steel", 2112, "A Farewell to Kings", "Hemispheres", "Permanent Waves", "Moving Pictures", "Signals", "Grace Under Pressure", "Power Windows", "Hold Your Fire", "Presto", "Roll the Bones", "Counterparts", "Test for Echo", "Vapor Trails", "Snakes & Arrows", "Clockwork Angels", "All the World's a Stage", "Exit...Stage Left", "A Show of Hands", "Different Stages"]

So when the sample is 2112, it tries to do 2112.match(%r{^\/})

Do you know if this is related to this PR? Any guidelines on how to move forward?

Thanks in advance!

(EDITED): seems to have run again the tests and now they are passing.

Opened a PR to solve this: #2050

@Zeragamba Zeragamba merged commit 6f8b25a into faker-ruby:master Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Faker::Number.decimal(l_digits: 1, r_digits: 1) always returns 0 in decimal part
3 participants