Skip to content
New issue

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

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

Already on GitHub? # to your account

Allow to pass a lambda function to sequence initial value #1434

Closed
wants to merge 1 commit into from

Conversation

javierav
Copy link

@javierav javierav commented Aug 4, 2020

This merge allow us to pass a lambda function as the initial value of sequence. This lambda will be called the first time that is used (lazy load). For example:

factory :user do
  sequence(:email, -> { User.count }) { |n| "person#{n}@example.com" }
end

@composerinteralia
Copy link
Collaborator

Thanks for the PR. This is an interesting idea. Can you provide some more detail on when this would be useful? For the -> { User.count } example you gave, is there a reason you need to start the sequence at that particular value vs. starting either at 1 or, if there are already some user fixtures loaded, at some arbitrary number like 1000?

end
end

def first_value
Copy link
Collaborator

Choose a reason for hiding this comment

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

Calling the block again when we rewind means that rewinding will not bring us back to the value we started at. This sort of goes against why we introduced the feature to rewind sequences - it was for people who were relying on specific ids in their tests for one reason or another.

If we go forward with this feature, I think we will need some additional test coverage around how this works with rewinding sequences.

@javierav
Copy link
Author

javierav commented Aug 13, 2020

Thanks you @composerinteralia for your comments. I review them ASAP, I do not have time these days....

@composerinteralia
Copy link
Collaborator

Thanks for the response. ASAP is not necessary at all. This PR isn't going anywhere, so please take your time.

@entcheva
Copy link

entcheva commented Dec 4, 2020

Hi @javierav - are you still interested in working on this, or should we close this PR? If you're still planning on working on this within the next few months, we'll keep it open, but if you no longer have the time to commit we can close it. Thank you!

@entcheva
Copy link

Hi @javierav - since there hasn't been activity on this PR since August, I'm going to close it. If you find you have time in the future to revisit, please feel free to reopen or open a new PR!

@entcheva entcheva closed this Jan 28, 2021
@aalvarado
Copy link

You can change the existing sequence in the registry itself:

FactoryGirl.sequences.find(:email).instance_variable_set(:@value, FactoryGirl::Sequence::EnumeratorAdapter.new(1000))

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants