-
-
Notifications
You must be signed in to change notification settings - Fork 16
[RFC] Add FactoryBot migration guide #139
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
base: main
Are you sure you want to change the base?
Changes from all commits
3c8f3ec
db65f82
88c445b
47eeced
2610c3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,239 @@ | ||||||||||||||||||||||
| # [WIP] Migrating from FactoryBot | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Thoughts/ideas on how to introduce Oaken into your codebase, either alongside or to ultimately replace FactoryBot. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Outline | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Get a handle on your object graph | ||||||||||||||||||||||
| - Identify root-level models | ||||||||||||||||||||||
| - Oaken is designed to mirror how your app works | ||||||||||||||||||||||
| - Identify pure "seed"/"reference data" (shared records) | ||||||||||||||||||||||
|
jbakerdev marked this conversation as resolved.
|
||||||||||||||||||||||
| - These could be candidates for seeding at the beginning of your test suite (if not already) | ||||||||||||||||||||||
| - Identify models with zero/one associations | ||||||||||||||||||||||
|
jbakerdev marked this conversation as resolved.
|
||||||||||||||||||||||
| - These could be the first refactors | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 2. Start using Oaken in development | ||||||||||||||||||||||
| - 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 | ||||||||||||||||||||||
|
Comment on lines
+16
to
+23
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 3. Testing - Use Oaken alongside FactoryBot | ||||||||||||||||||||||
| - Configure Oaken with your test suite | ||||||||||||||||||||||
| - Add `require "oaken/rspec_setup"` to `rails_helper.rb` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - Idea 1: Seed shared records at the beginning of the test run | ||||||||||||||||||||||
| - Rationale: | ||||||||||||||||||||||
| - Simplify test setup | ||||||||||||||||||||||
| - Consolidate dev/test data | ||||||||||||||||||||||
|
Comment on lines
+29
to
+32
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should be expanded a bit further with some concrete examples. I agree, and think the following could be good candidates:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We have some things in the README that tries to establish a convention in a
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Examples are also really hard in general. I have been wondering if there's a quickstart in saying 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:
|
||||||||||||||||||||||
| - Suggested approach: | ||||||||||||||||||||||
| - Configure `seeds.rb` with handling for test environment | ||||||||||||||||||||||
| - If that breaks tests: | ||||||||||||||||||||||
| - Check for redundant setup actions | ||||||||||||||||||||||
| - Are there let!/before blocks that you can remove? | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - Idea 2: Bridge the gap - Start using Oaken's factory-like features with existing specs | ||||||||||||||||||||||
| - Rationale: | ||||||||||||||||||||||
| - Specs may only require trivial changes | ||||||||||||||||||||||
| - Simple factories translate well to Oaken defaults | ||||||||||||||||||||||
| - Simple traits translate well to Oaken helpers | ||||||||||||||||||||||
| - Suggested approach: | ||||||||||||||||||||||
| - Create new defaults/helper methods | ||||||||||||||||||||||
| - Use in the same way/places as factories (inside let/before blocks) | ||||||||||||||||||||||
| - build -> `s/build(:thing, ...)/things.build(...)/` | ||||||||||||||||||||||
| - create -> `s/create(:thing, ...)/things.create(...)/` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - Idea 3: Start using Oaken for new features | ||||||||||||||||||||||
| - Rationale: | ||||||||||||||||||||||
| - YMMV, but a new feature may be a good opportunity to start using Oaken | ||||||||||||||||||||||
| - This is trivial if the new feature is introducing a new root model | ||||||||||||||||||||||
| - This is still reasonable if your feature is only loosely coupled with the existing models/factories | ||||||||||||||||||||||
| - Suggested approach: | ||||||||||||||||||||||
| - Create Oaken fixtures (try out the features) | ||||||||||||||||||||||
| - Try out labels? | ||||||||||||||||||||||
| - Maybe you need to organize into cases? | ||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading your comments and then re-reading my outline, i agree, this needs some work. Backing up, my intent was twofold:
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:
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)
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oops somehow I missed this, when I was going through comments the other day!
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:
Which should ideally be less than an hour of initial investment? Then the follow up could be:
I don't know, something like that maybe?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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.
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 😂 |
||||||||||||||||||||||
| - Create new defaults/helper methods | ||||||||||||||||||||||
| - Update seeds.rb | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 4. The last 10% | ||||||||||||||||||||||
| - **WIP** | ||||||||||||||||||||||
| - Not 100% sure what this should contain | ||||||||||||||||||||||
| - In my own codebases I'm starting to re-think how I test certain things. | ||||||||||||||||||||||
| - Maybe this could contain a "cookbook" of sorts, with recipes for refactoring Rspec/FactoryBot paradigms that don't quite translate to Oaken? | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Get a handle on your object graph | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Oaken helps you build an object graph that reflects your domain model. So for best results, spend some time upfront getting (re)acquainted with your app's models and how they relate to each other. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Granted, that's usually easier said than done... so, where does one start? | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 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. | ||||||||||||||||||||||
|
Comment on lines
+74
to
+78
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||||||||||||||||||||||
| - Secondary: models that hang off the primary ones, like Sessions for a User, Invites for an Account | ||||||||||||||||||||||
| - Tertiary: everything else that may hang off of primary + secondary models, probably a good case for factories | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| The lines between these categories can be pretty blurry, so treat this like a framing device rather than a rule. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### Tip: Explore the object graph using the Rails console | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Here's a way to identify models with few associations using the Rails console | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
| # Enable eager loading | ||||||||||||||||||||||
| dummy(dev)> Rails.application.eager_load! | ||||||||||||||||||||||
| => nil | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Get number of total models | ||||||||||||||||||||||
| dummy(dev)> ApplicationRecord.descendants.size | ||||||||||||||||||||||
| => 8 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # List all models | ||||||||||||||||||||||
| dummy(dev)> ApplicationRecord.descendants | ||||||||||||||||||||||
| => | ||||||||||||||||||||||
| [Menu::Item::Detail (call 'Menu::Item::Detail.load_schema' to load schema informations), | ||||||||||||||||||||||
| Menu::Item (call 'Menu::Item.load_schema' to load schema informations), | ||||||||||||||||||||||
| User (call 'User.load_schema' to load schema informations), | ||||||||||||||||||||||
| Plan (call 'Plan.load_schema' to load schema informations), | ||||||||||||||||||||||
| Order (call 'Order.load_schema' to load schema informations), | ||||||||||||||||||||||
| Menu (call 'Menu.load_schema' to load schema informations), | ||||||||||||||||||||||
| Administratorship (call 'Administratorship.load_schema' to load schema informations), | ||||||||||||||||||||||
| Account (call 'Account.load_schema' to load schema informations)] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # List models with a single association | ||||||||||||||||||||||
| dummy(dev)> ApplicationRecord.descendants.select { _1.reflect_on_all_associations.size < 2 } | ||||||||||||||||||||||
|
Comment on lines
+109
to
+110
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||||||||||||||||||
| => | ||||||||||||||||||||||
| [Menu::Item::Detail (call 'Menu::Item::Detail.load_schema' to load schema informations), | ||||||||||||||||||||||
| Menu::Item (call 'Menu::Item.load_schema' to load schema informations), | ||||||||||||||||||||||
| Plan (call 'Plan.load_schema' to load schema informations)] | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## (WIP) Gotchas and things that work differently | ||||||||||||||||||||||
|
jbakerdev marked this conversation as resolved.
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| > FYI - These are "gotchas" from my personal experience. I'm not 100% sure how to organize these. | ||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if these could be styled as an FAQ somewhere? |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Oaken's factory-esque features can frequently be a drop-in replacement for FactoryBot, but nevertheless, there are some differences that FactoryBot users should be aware of. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### Sequences | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| FactoryBot sequences can generally be replaced with Ruby's `Numeric#step` captured in local variables that you pass to blocks in `defaults`. Like this: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||
| # db/seeds/setup.rb | ||||||||||||||||||||||
| user_count, email_address_count = 0.step, 0.step | ||||||||||||||||||||||
| users.defaults name: -> { "Customer #{user_count.next}" }, | ||||||||||||||||||||||
| email_address: -> { "email_address#{email_address_count.next}@example.com" } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Or set them globally on the loader if it's safe to do so: | ||||||||||||||||||||||
| loader.defaults name: -> { "Customer #{user_count.next}" }, | ||||||||||||||||||||||
| email_address: -> { "email_address#{email_address_count.next}@example.com" } | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### 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. | ||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, you don't really choose between build and create upfront, you're really being asked to just What do you think about this? I'm slightly mixing in some selling points in here, but we could yank those.
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **build_stubbed** | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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? | ||||||||||||||||||||||
|
Comment on lines
+144
to
+147
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just looked up what E.g. 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 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with Kasper here. The beauty of Oaken's approach is the removal of the edge-cases & complexities of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at this again, I do like the idea of supporting |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| > **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... | ||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, really interesting! Do you remember what the difference was?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'd be really curious to hear more, and maybe there's something we can learn/add from that :) |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ### Associations | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Build strategies become more important with associated models. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **FactoryBot** | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| One generally just declares the association: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||
| # user_factory.rb | ||||||||||||||||||||||
| factory :user do | ||||||||||||||||||||||
| account # declared once in factory | ||||||||||||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure where this fits but one other idea I had was to suggest doing |
||||||||||||||||||||||
| end | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| The build strategy is specified by the calling code (eg, the spec) and cascades through all associated factories/models automatically. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||
| # Usage - account auto-created/built based on strategy | ||||||||||||||||||||||
| create(:user) # creates user AND account | ||||||||||||||||||||||
| build(:user) # builds user AND account (both non-persisted) | ||||||||||||||||||||||
| build_stubbed(:user) # stubs both | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **Oaken** | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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... | ||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feel free to pull from https://github.com/practical-computer/flip-on-the-awesome-snapshot! |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. Accept explicitness in tests | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||
| # spec | ||||||||||||||||||||||
| it "tests non-persisted records" do | ||||||||||||||||||||||
| account = accounts.build | ||||||||||||||||||||||
| user = users.build(accounts: [account]) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # expect(user).to eq(...) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it "tests persisted records" do | ||||||||||||||||||||||
| account = accounts.create | ||||||||||||||||||||||
| user = users.create(accounts: [account]) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # expect(user).to eq(...) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 2. Create separate helpers | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ruby | ||||||||||||||||||||||
| # db/seeds/setup.rb | ||||||||||||||||||||||
| def users.build_with_account(label = nil, accounts: [accounts.build], **) | ||||||||||||||||||||||
| build(label, accounts:, **) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| def users.create_with_account(label = nil, accounts: [accounts.create], **) | ||||||||||||||||||||||
| create(label, accounts:, **) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # spec | ||||||||||||||||||||||
| it "tests non-persisted records" do | ||||||||||||||||||||||
| user = users.build_with_account | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # expect(user).to eq(...) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| it "tests persisted records" do | ||||||||||||||||||||||
| user = users.create_with_account | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # expect(user).to eq(...) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 3. Rethink the spec | ||||||||||||||||||||||
| - Does the spec actually need a non-persisted record? | ||||||||||||||||||||||
| - Is build used out of habit rather than necessity? | ||||||||||||||||||||||
| - Is this worth refactoring to use seeded records? | ||||||||||||||||||||||
| - Reference a pre-seeded user/account instead of building one per-test? | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 4. Keep the factory? | ||||||||||||||||||||||
| - Oaken and FactoryBot can coexist... | ||||||||||||||||||||||
|
Comment on lines
+233
to
+234
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is part of what I wanted to address with my idea 3, maybe that content is better here?
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that idea 3 sounds like an interesting place to flesh this out more. |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| **The tradeoff:** | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - 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 | ||||||||||||||||||||||
|
Comment on lines
+238
to
+239
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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!
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tcannonfodder good call on "declarative", yeah! |
||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.