-
Notifications
You must be signed in to change notification settings - Fork 41
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
New versions of FactoryBot don't like us setting IDs by hand #707
Comments
It seems that `factory_bot_rails` unexpectedly introduced a breaking change. thoughtbot/factory_bot#1602 For now I'm just going to avoid that version while we decide what to do about it: bullet-train-co/bullet_train-core#707
For now I'm just going to sidestep that upgrade so we can let depfu keep doing it's thing while we figure out what to do: bullet-train-co/bullet_train#1210 |
It seems that `factory_bot_rails` unexpectedly introduced a breaking change. thoughtbot/factory_bot#1602 For now I'm just going to avoid that version while we decide what to do about it: bullet-train-co/bullet_train-core#707
Found the PR where the new thing was introduced, and I mentioned our (unusual) use case there. Don't know if anything will come of it, just wanted to drop a link: thoughtbot/factory_bot_rails#419 |
Well, we use FactoryBot to create examples for OpenAPI schema, therefore we need it in production. I agree, that is an unusual way to use it. I see another option to avoid this new error: by reverting back the PR where we switched from .create to .build. Not sure how can we replace FactoryBot in production. We need examples to reflect the real records, but the real DB may be empty, and we should not expose the real data in API documentation. I will research for the other ways to create dummy objects mimicking the real records, but not sure there exists anything besides things like FactoryBot which are meant for testing purposes. Maybe we could write something by ourselves, as we know the DB schema and relations from ActiveRecord, so that should be a possible task to create some objects with type-specific data (like "Text" for strings and "42" for integers). Don't know if it's worth it though. |
@jagthedrummer looks like the FactoryBot is planning on softening the limit in a later release thoughtbot/factory_bot_rails#419 (comment) So we may not need to change anything here anymore, than just disable whatever the lint/flag they add is gonna be. |
Yeah, I think we just wait it out and see how things go once that change is made. For now we can just stick to an older version. |
FYI it's been fixed https://github.com/thoughtbot/factory_bot_rails/releases/tag/v6.4.3. |
I think it was fixed in the sense that it reverts back to the old use before sequence(:id, intentionally_override_auto_populated_database_column: true) { |n| "#{n}#{n}" } |
@gazayas good point, I think this sounds like a good call to me. |
I think switching to native Rails fixtures might be a right move, that would allow drop dependancy on FactoryBot we actually don't need. |
On this depfu PR that's trying to update
factory_bot
andfactory_bot_rails
in the starter repo we started to see this error:There's an open issue about it, and the maintainers seem to be saying that it's a desired breaking change (though it's not behind a major version bump) and that the way to fix it is to set a new config option.
I think there are two ways forward here.
config/environments/development.rb
in the starter repo.For ease I like the easy way, but it kinda leaves a bad taste in my mouth for a few reasons.
One of those reasons is that by setting that config option we're straying off of the normal path for FactoryBot, and I'm not sure that we want to dictate the leaving of that path for downstream developers (maybe we do?). There really wouldn't be a viable way for people to be able to get back onto the normal path AND to test their Open API stuff. They'd have to choose one or the other. Also, I'm generally not a huge fan of
factory_bot
being in theproduction
group (it's currently in that group by way of being a hard, non-development dependency in thebullet_train-api
gem). It really feels like we're kind of misusing FB, or at least using it in unintended ways. I'd love to get it back to being just a development dependency instead of also being a deployment dependency.So, for those reasons I like the hard way. But I don't really know what it would take to get FactoryBot disentangled from ExampleBot.
@newstler, @gazayas, @kaspth, what do you guys think?
The text was updated successfully, but these errors were encountered: