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

[Backwards Breaking change 2.22] Faker is reliably generating duplicate values #2534

Closed
danlo opened this issue Aug 12, 2022 · 10 comments · Fixed by #2579
Closed

[Backwards Breaking change 2.22] Faker is reliably generating duplicate values #2534

danlo opened this issue Aug 12, 2022 · 10 comments · Fixed by #2579

Comments

@danlo
Copy link

danlo commented Aug 12, 2022

Bug report

Versions

Faker 2.22 (Only this one, tested 2.20, 2.21), Rails 7.0.3.1
Ubuntu 20.04 LTS

Describe the bug

Sets of values are being generated in a non-random fashion.

To Reproduce

Execute the following code from a rails command line.

Rails 7.0.3.1 command line

clear; rails test test/rand1_test.rb test/rand2_test.rb

test/rand_test1.rb

# frozen_string_literal: true

require 'test_helper'

# clear; rails test test/rand_test.rb

class RandTest < ActiveSupport::TestCase
  test 'test #1' do
    $GLOBAL_CHECK={} unless defined?($GLOBAL_CHECK)
    $GLOBAL_CHECK.default = 0

    20.times do
      key = "#{Faker::Name.first_name} #{Faker::Name.last_name} #{Faker::Name.last_name}"
      $GLOBAL_CHECK[key] +=1
      puts "Duplicated1: #{key}" if $GLOBAL_CHECK[key] > 1
      puts "Test#1: #{key}"
    end
  end
end

test/rand_test2.rb

# frozen_string_literal: true

require 'test_helper'

# clear; rails test test/rand_test.rb

class Rand2Test < ActiveSupport::TestCase
  test 'test #2' do
    $GLOBAL_CHECK={} unless defined?($GLOBAL_CHECK)
    $GLOBAL_CHECK.default = 0

    20.times do
      key = "#{Faker::Name.first_name} #{Faker::Name.last_name} #{Faker::Name.last_name}"
      $GLOBAL_CHECK[key] +=1
      puts "Duplicated2: #{key}" if $GLOBAL_CHECK[key] > 1
      puts "Test#2: #{key}"
    end
  end
end

Expected behavior

Data is truly randomly generated

Additional context

This is not happening in 2.21 and only is reproducible in 2.22

The requirements for this to happen, is there MUST BE 2 rails tests (individual files).

This is due to this patch: 04d2703

I confirmed this by appending this code to the end of my test_helper.rb to monkey patch test it. I would remove the .new on end of Random and try it.

require 'faker'

module Faker
  module Config
    class << self
      def random
        @random || Random.new
      end
    end
  end
end

Suspicion(s)

I suspect, the issue, may be that the Rails test code is re-seeding the Random with the same seed at the beginning of every test case. Previously using Random.new gave Faker it's own random number generator, which was setup for the entire run of the test suite.

@danlo danlo changed the title Faker is generating duplicate values (very un-randomly) [Backwards Breaking change] Faker is generating duplicate values (very un-randomly) Aug 12, 2022
@danlo danlo changed the title [Backwards Breaking change] Faker is generating duplicate values (very un-randomly) [Backwards Breaking change] Faker is reliably generating duplicate values Aug 12, 2022
@danlo danlo changed the title [Backwards Breaking change] Faker is reliably generating duplicate values [Backwards Breaking change 2.22] Faker is reliably generating duplicate values Aug 12, 2022
@danlo
Copy link
Author

danlo commented Aug 12, 2022

@sudeeptarlekar Flagging this to for your attention

@sudeeptarlekar
Copy link
Contributor

This was fixed as a part for this issue #2487.

In this case as we are running two test files, which generates the reproducible random values and looks valid, but let's hear out other's thought about this. Thanks for pointing this out.

What are your thoughts @stefannibrasil @psibi @Zeragamba @koic ?

@danlo
Copy link
Author

danlo commented Aug 13, 2022

For those that are curious, this came about because my code checks for double entries. If the first_name and last_name are repeated with in the last X minutes, then it flags it as a double entry. Because the "random" values re-appear from test case to test case, the code correctly... or incorrectly... notices the duplicate values and barfs.

@sudeeptarlekar
Copy link
Contributor

May be we can add some kind of config, thoughts?

@danlo
Copy link
Author

danlo commented Aug 18, 2022

While, this is being figured out, would the repo accept a README.markdown update to notify people of the change of behavior for rails tests? (So others do not run into this same problem?)

@Zeragamba
Copy link
Contributor

would the changes being proposed by #2471 help resolve this issue?

@sudeeptarlekar
Copy link
Contributor

sudeeptarlekar commented Sep 12, 2022

Checking the changes you mentioned @Zeragamba, but meanwhile can't we add following line to test_helper.rb or rails_helper.rb to reset random object

Faker::Config.random = Random.new

After adding above code in test and rails helpers, I am not getting duplicate error anymore.

What are your thoughts @danlo?

@stefannibrasil
Copy link
Contributor

Thank you for reporting this issue and providing the reproduction steps @danlo. Sorry I missed this for a while.

Just out of curiosity, I was not able to reproduce this bug with RSpec, only with Minitest. Since this bug report seems to happen only on a very specific case, I'd vote for adding this to the documentation.

Adding Faker::Config.random = Random.new to the test_helper.rb file fixes this for Minitest and Faker > 2.22 users. This would avoid breaking Faker for most users. Thanks @sudeeptarlekar for suggesting that.

@danlo are you okay with opening a PR to add the following extra configuration for this scenario?

@stefannibrasil
Copy link
Contributor

Please review #2579 so we can close this issue. Thank you!

@danlo
Copy link
Author

danlo commented Oct 5, 2022

Thank you. I've been wanting to handle this however, work has been hard on me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants