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

Use build in ExampleBot instead of create + rollback #524

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

jagthedrummer
Copy link
Contributor

@jagthedrummer jagthedrummer commented Sep 5, 2023

Joint PR: bullet-train-co/bullet_train#965

This PR makes it so that we use FactoryBot.build when generating example records in ExampleBot.

Doing this so that we don't have to actually create live records, clone them, rollback, then reset primary keys. Doing all of that work is slow so it's best to avoid doing it. (And having to reset primary keys for something that we expect to be a normal thing kind of gives me the willies.)

@jagthedrummer jagthedrummer changed the title WIP: Can we use build instead of create + rollback Use build in ExampleBot instead of create + rollback Sep 5, 2023
@jagthedrummer jagthedrummer marked this pull request as ready for review September 5, 2023 21:22
@jagthedrummer
Copy link
Contributor Author

@newstler, you mentioned that making this change would actually break some things that we don't have tests for. Can you provide an example of something that would break? I'd like to look into getting the needed tests in place, but I don't really know what I'm looking for.

@newstler
Copy link
Contributor

newstler commented Oct 3, 2023

Hey @jagthedrummer, yep I remember about this, and I started writing tests for it. Just got busy with fixing bugs in jbulder-schema and bullet_train-api. How urgent is this?

@jagthedrummer
Copy link
Contributor Author

It's not super urgent, but I would like to keep it moving. Can you give me an idea of what you know about that breaks with this change? I literally have zero idea why this is problematic in any way.

@newstler
Copy link
Contributor

newstler commented Oct 3, 2023

I also have to make some investigation, will try to do it this week.
Basically the OpenAPI result should be the same for both cases, my idea is to test it on some real project with a significant amount of models and inter-model relations, and see what's going on there with callbacks and write tests for it.

@newstler
Copy link
Contributor

newstler commented Oct 9, 2023

@jagthedrummer Answered in the starter repo bullet-train-co/bullet_train#965 (comment)

@jagthedrummer jagthedrummer merged commit a20d4cd into main Nov 1, 2023
5 checks passed
@jagthedrummer jagthedrummer deleted the jeremy/example-bot-build branch November 1, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants