[RFC] Add FactoryBot migration guide#139
Conversation
| - Suggested approach: | ||
| - Create Oaken fixtures (try out the features) | ||
| - Try out labels? | ||
| - Maybe you need to organize into cases? |
There was a problem hiding this comment.
Yeah this is interesting, I guess there's a difference here between a bottom-up approach described here and then the refactor seeds first top-down approach.
There was a problem hiding this comment.
This idea feels the most brittle; because it doesn't solve immediate, common needs across the entire system for folks (compared to Idea 1). That might cause folks to balk or bristle at Oaken being a "superfluous change", without seeing some of Oaken's best features.
There was a problem hiding this comment.
Reading your comments and then re-reading my outline, i agree, this needs some work.
Backing up, my intent was twofold:
- Show a way of using Oaken for new work (rather than just refactoring)
- Reinforce the idea that Oaken can coexist with Factorybot
Perhaps my example isn't the best way to illustrate this, and that's fine (I'm a solo dev on this particular project, which gives me options that a team doesn't necessarily have)
I should clarify:
-
I had already done the leg work outlined in ideas 1&2, which made it feasible to build a new feature using Oaken. So idea 3 likely can't be implanted on its own.
-
I was starting a new feature (what the client wanted) but I was keen to use seeds/fixtures from the start (rather than continue on with factories and replace at a later time) It just so happened that this feature was introducing a new root/primary model and was largely independent of the existing object graph.
So in my case this approach worked out, which is especially great because my clients never appreciate paying to refactor "working code," but... explaining it after the fact, this does read like a contrived example.
And paraphrasing @tcannonfodder, I definitely don't want this to sound like a superfluous change, or for people to "bristle" (great word!) at the thought of introducing a disparate/one-off approach that could potentially never become the new default... so perhaps these column inches would be better allocated for techniques to do just that (implement Oaken across the app)
There was a problem hiding this comment.
oops somehow I missed this, when I was going through comments the other day!
Backing up, my intent was twofold:
- Show a way of using Oaken for new work (rather than just refactoring)
- Reinforce the idea that Oaken can coexist with Factorybot
Yeah I definitely like this idea.
I also just heard from someone that started by converted a single model spec and saw a runtime drop from ~14 seconds to ~5 seconds. And then they're continuing on to try to convert some larger slower specs and see what that the results are there. I think that's really interesting as an isolated way to wet the appetite and see if there's any results, and then go for more.
So that's essentially:
bundle add oakenrequire "rspec_setup"- Find a small to medium model spec that's slow enough to see a benefit
- Extract a
createfor the reused model intodb/seeds/test/<model>s.rbor similar Oaken.seed <model>indb/seeds.rb(can even wrap it in anif Rails.env.test?to reduce blast radius)- Coerce/fix specs as needed
- Compare run times by running the single spec
- Continue extracting setup as needed and see if you can improve the run times
Which should ideally be less than an hour of initial investment?
Then the follow up could be:
- Find another same-size or slightly bigger spec
- Reuse the models you've already set up, or add new ones as needed
- This is where the shared data setup can get tricky, so it's useful to pay attention to how confusing/clear that's starting to feel.
I don't know, something like that maybe?
There was a problem hiding this comment.
I should clarify:
- I had already done the leg work outlined in ideas 1&2, which made it feasible to build a new feature using Oaken. So idea 3 likely can't be implanted on its own.
- I was starting a new feature (what the client wanted) but I was keen to use seeds/fixtures from the start (rather than continue on with factories and replace at a later time) It just so happened that this feature was introducing a new root/primary model and was largely independent of the existing object graph.
So in my case this approach worked out, which is especially great because my clients never appreciate paying to refactor "working code," but... explaining it after the fact, this does read like a contrived example.
Hmm, I do like this flow though. And I'm glad that that worked out for the client too, you could kinda trojan horse it while working on the feature.
And paraphrasing @tcannonfodder, I definitely don't want this to sound like a superfluous change, or for people to "bristle" (great word!) at the thought of introducing a disparate/one-off approach that could potentially never become the new default... so perhaps these column inches would be better allocated for techniques to do just that (implement Oaken across the app)
Yeah I guess it's like how can we give people a fairly representative sample of what the rest of their app would look like on Oaken? Pretty tough though 😂
|
@jbakerdev thank you, this is great! I'll leave some more detailed comments on the rest of things in a bit. |
| ```ruby | ||
| # user_factory.rb | ||
| factory :user do | ||
| account # declared once in factory |
There was a problem hiding this comment.
I'm not sure where this fits but one other idea I had was to suggest doing account { Oaken.context.accounts.kaspers_donuts }. So you seed that one account and use that by default. Hopefully no tests/spec fail though.
| Thinking out loud - Is there a comparable approach for build_stubbed with Oaken? | ||
|
|
||
| - I'm thinking that there isn't (and that there probably shouldn't be) because stubbing isn't what Oaken is all about | ||
| - And... perhaps that's a good thing? |
There was a problem hiding this comment.
I just looked up what build_stubbed is and I don't think it makes sense for Oaken. Users can replace build_stubbed with probably a single seeded record and it'll be way less complexity for them and for me to write/maintain (because there's nothing extra to write/maintain).
E.g. users.kasper will behave like build_stubbed(:user), and users.kasper.update! works for cases where you pass attributes. We've also got users.build.
I don't know if there's other things I'm missing with the idea of passing stubs around? Ensuring that users can't save a passed record? Maybe users.kasper.tap(&:readonly!) helps with that? (implementation note: maybe we can do proxy :readonly and get users.readonly.kasper working out of the box?)
Don't know where this fits: part of the hypothesis with Oaken is that seeding the 10-20 objects that you use throughout your tests removes a lot of the complexity that FactoryBot needs.
There was a problem hiding this comment.
I agree with Kasper here. The beauty of Oaken's approach is the removal of the edge-cases & complexities of build/build_stubbed/create in favor of a straightforward path, while still maintaining (approximately) similar performance of fixtures.
There was a problem hiding this comment.
Thanks gents, I especially appreciate you chiming in on this one. I don't feel like enough of an expert to argue for/against the utility of build_stubbed... but my gut feeling was that it didn't make sense to try to replicate in oaken.
There was a problem hiding this comment.
Looking at this again, I do like the idea of supporting users.readonly.kasper so you can be sure code doesn't touch the database if you don't want to. I think there's something there for a separate PR.
| - I'm thinking that there isn't (and that there probably shouldn't be) because stubbing isn't what Oaken is all about | ||
| - And... perhaps that's a good thing? | ||
|
|
||
| > **Personal anecdote:** When migrating some factories/specs i found a couple of cases where `build_stubbed` didn't behave as i expected, and my specs weren't testing what I thought they were... |
There was a problem hiding this comment.
Wow, really interesting! Do you remember what the difference was?
There was a problem hiding this comment.
Ugh I know I didn't make a note of this, but I think I can dig it up from the commit history (away on vacation now) it could very well be an example of me using factorybot improperly, but perhaps that could be worth including in this doc as why build_stubbed can be problematic?
There was a problem hiding this comment.
Yeah, I'd be really curious to hear more, and maybe there's something we can learn/add from that :)
| - FactoryBot: More flexibility, but associations can cascade unexpectedly (one factory pulls in the whole app) | ||
| - Oaken: More code, but more explicit. You always know what's being built/created |
There was a problem hiding this comment.
Maybe this is my bias showing but my contention is that Oaken is just as flexible (if not more), and will overall result in less code. If it's looking like it's more code, my rough sense is it's a sign the user is trying to replicate FactoryBot with Oaken.
There was a problem hiding this comment.
I would maybe say it's "more declarative", rather than "more code". I know my oaken seeds are a bit longer than what a factory looks like, but it's much more scannable & I know the state of a seed by looking at a single file versus comparing multiple factories
There was a problem hiding this comment.
@kaspth i was struggling to summarize this - i agree and I think your comment that Oaken "will overall result in less code" is one of the arguments I should be making here.
@tcannonfodder ahhh yes, "declarative" is the sort of word I was searching for, nice one!
There was a problem hiding this comment.
I guess there's two things I'm contending when comparing to factories: overall I think apps will ship less code, and for the places where there's more code than factories it'll be clearer or more easily scannable.
For instance, one team that swapped to Oaken was initially skeptical but then they started shaving off 20 lines of setup code per test case in general, and their tests were faster too.
| - Idea 1: Upgrade existing development seeds with Oaken | ||
| - Rationale: | ||
| - Oaken is worth using just for the seeding features in development | ||
| - Personal anecdote: I got many of the benefits of the Seed-Fu gem | ||
| - Probably already have seeds that you can streamline/optimize | ||
| - Personal anecdote: I replaced a complicated seeding task with Oaken | ||
| - You can stop here and still be better off than before you started with Oaken | ||
| - No one says you have to use Oaken for tests, but this could be the gateway drug |
There was a problem hiding this comment.
I think this is a strong rationale, and definitely should be emphasized as part of the migration from FactoryBot. Folks can get afraid of changing away from Factories because they feel so ingrained, so if you attack the problem from an oblique angle, you defuse the situation and folks can see the benefits of Oaken
There was a problem hiding this comment.
@jbakerdev I definitely like what you've got here with having several angles of approach, so users can kinda pick from what's their biggest pain point and start there and see if it's easier/better.
I've also tried to list out some general benefits and specific ones over factories in the README now, so there's potentially something extra to pull from there too https://github.com/kaspth/oaken?tab=readme-ov-file#benefits
| - Idea 1: Seed shared records at the beginning of the test run | ||
| - Rationale: | ||
| - Simplify test setup | ||
| - Consolidate dev/test data |
There was a problem hiding this comment.
Maybe this should be expanded a bit further with some concrete examples. I agree, and think the following could be good candidates:
- Root-level models
- "Meta" models, such as plans or feature sets
- your trickiest edge case setups, where models need to be arranged just right to test a specific case. This is where Oakens'
test/casesbehavior is useful!
There was a problem hiding this comment.
Re: examples, I concur and frankly this whole doc needs better examples.
Re: edge cases, I was originally thinking of cases as a more in depth topic that would be better saved for later. (I haven't personally used them on my project, the code that needs them is still heavily factory-based) But if cases solve problems at this stage then why not introduce them right away? (Help demo Oaken strengths early on)
There was a problem hiding this comment.
"Meta" models, such as plans or feature sets
We have some things in the README that tries to establish a convention in a db/seeds/data folder. They're sorta their own root-level models that's loosely coupled to the rest of the app, but other models will want to pull them in, e.g. the recommendation is Oaken.seed :data, :accounts.
There was a problem hiding this comment.
Re: examples, I concur and frankly this whole doc needs better examples.
Examples are also really hard in general. I have been wondering if there's a quickstart in saying create(:account) maps to accounts.create and same for build.
Then for associations, you generally want to build a scenario and explicit names as needed. E.g. user doesn't make an order, it's Sally that purchases a Plain Donut costing $3 from the Little Glaze Boutique and is serviced by John. Making it vivid makes easier to remember and learn your app throughout your test cases.
Then traits map to:
- Just writing out the scenario and take advantage of the preseeding for tests making it fast.
- For more obscure traits there's also helpers, so
create(:account, :some_trait)could map toaccounts.some_trait(helpers are more flexible so there's more choice and I'm less sure of a recommendation). - There's also the
db/seeds/casesconvention, which could becases/performance.rbandcases/performance/pagination.rb, you generally want to seed them as needed either in specific test cases via justseed "cases/performance/pagination"or in the development console viaOaken.seed "cases/performance"orOaken.seed "cases/performance/pagination".
| 4. Keep the factory? | ||
| - Oaken and FactoryBot can coexist... |
There was a problem hiding this comment.
I also think this is worth emphasizing. Some codebases/test suites/tech budgets are too small to replace it outright, but Oaken can run in tandem
There was a problem hiding this comment.
This is part of what I wanted to address with my idea 3, maybe that content is better here?
There was a problem hiding this comment.
Yeah, that idea 3 sounds like an interesting place to flesh this out more.
|
|
||
| Associations are always explicit. So if a spec relies on both build(:user) and create(:user), you can: | ||
|
|
||
| **WIP** I think this would benefit from some less trivial examples... |
There was a problem hiding this comment.
Feel free to pull from https://github.com/practical-computer/flip-on-the-awesome-snapshot!
- I like the idea of primary/secondary/tertiary models, lets run with this
- Want to keep emphasizing that Oaken is meant to help mirror how the app works
kaspth
left a comment
There was a problem hiding this comment.
Really liking where this is going!
| ### Categorizing models | ||
|
|
||
| It can be helpful to break down the object graph by grouping models into categories: | ||
|
|
||
| - Primary: a root-level model like Account and it's Users with 1-4 other models, the most basic crucial data to have to bootstrap the app. |
There was a problem hiding this comment.
I wonder if the first step is identifying the root-level model? Because it can be a really helpful framing device for everything else.
And then we can give introduce Primary, Secondary, Tertiary language, and for Primary say: starts with your identified root-level model, and then the 1-4 other models that's crucial to boot the app. Common examples include User.
Maybe another way to help people find this is: what models are there after a new account sign up?
| # List models with a single association | ||
| dummy(dev)> ApplicationRecord.descendants.select { _1.reflect_on_all_associations.size < 2 } |
There was a problem hiding this comment.
Technically this lists models with zero or 1 association. We could probably split this out like:
ApplicationRecord.descendants.select { _1.reflect_on_all_associations.size == 0 }ApplicationRecord.descendants.select { _1.reflect_on_all_associations.size == 1 }And then users could just keep bumping the N to find that N association count. That's pretty easy to just press arrow up in the and tweak the integer.
|
|
||
| ## (WIP) Gotchas and things that work differently | ||
|
|
||
| > FYI - These are "gotchas" from my personal experience. I'm not 100% sure how to organize these. |
There was a problem hiding this comment.
I wonder if these could be styled as an FAQ somewhere?
|
|
||
| ### Build Strategies | ||
|
|
||
| While FactoryBot offers build strategies as a feature, Oaken is more explicit by design, requiring the developer to choose between build and create upfront. This can seem frustrating when attempting to replicate factories that one would typically use with build AND/OR create AND/OR build_stubbed. |
There was a problem hiding this comment.
Hm, you don't really choose between build and create upfront, you're really being asked to just create and reuse the record.
What do you think about this? I'm slightly mixing in some selling points in here, but we could yank those.
| While FactoryBot offers build strategies as a feature, Oaken is more explicit by design, requiring the developer to choose between build and create upfront. This can seem frustrating when attempting to replicate factories that one would typically use with build AND/OR create AND/OR build_stubbed. | |
| In FactoryBot you define a factory that you can then convert to an actual object via any of `build`/`create`/`build_stubbed` later. | |
| By contrast, Oaken is declarative upfront by design. You most often `create` a record once before tests run and then reuse that throughout your tests. So you amortize the `create` cost against reusing it throughout 50+ tests. | |
| As a result, you no longer have to be an expert in `build`/`create`/`build_stubbed`, or remember when to use `build`/`build_stubbed` over `create`. That compounds across a team too. | |
| So instead, you can create records with labels to refer back to them, e.g. `accounts.create :small_co, name: "Small co."` letting tests call `accounts.small_co` which always returns a new instance. | |
| That said, Oaken still has `build` for the remaining 1-5% of cases where it's useful. |
I've been working on integrating on Oaken into my codebase for a few months now, and while the current documentation is very complete, I often found myself wishing for documentation focused on migrating away from FactoryBot.
So I tried to clean up my own notes/thoughts from working on my own projects, and see if I could generalize them a bit. I also brainstormed on a section of what works differently in Oaken, and some approaches I personally took to address that. I don't presume to be an Oaken expert at this point - hopefully I'm not overlooking something obvious!
Anyways... this is very much a WIP, but I wanted to put this up as a draft for comments/suggestions.