Fix class level loading #70
Conversation
lib/oaken/seeds.rb
Outdated
| delegate :entry, to: :loader | ||
| class << self | ||
| # Expose a class `seed` method for individual test classes to use. | ||
| # TODO: support parallelization somehow. |
There was a problem hiding this comment.
So I think this approach actually is the best way forward for supporting parallelization. My reason being that it's called on a per-test basis. Tests are already broken up into individual units of work from a parallelization perspective. Having the seed method be called on a per-test basis (or in a setup/before block, which is also called on an individual unit of work) helps reinforce test isolation. That:
- Reduces the amount of junk data that's generated for unrelated tests
- Makes it easier to debug a particular test
- Reduces test flakiness
- Encourages writing seed files for specific edge-case scenarios
There was a problem hiding this comment.
@tcannonfodder I added this as a recommendation and explanation in 6ed2503
There was a problem hiding this comment.
I think the README needs a general rinse but I liked the list you had and wanted that in there now. Though the writing I ended up with reads a bit like people should always have per-test seed files, which is not always the case.
When doing a `self.class.seed` from within a test, we'd be within the Test Class scope, so the `@loader` variable would assign there and and not on `Oaken::Seeds.loader`. The old structure was a little muddy anyway, so this attempts a rewrite that's hopefully clearer. Now we'll always be within `Oaken::Seeds`' `singleton_class`. Note: originally the point about injecting `load_onto Oaken::Seeds` was so you could potentially have other seeds modules, so you could have your app seeds split up ala Packwerk. But it doesn't really work and it's kinda unclear how I'd make it work, so it's probably better to yank it in a bit.
kaspth
force-pushed
the
fix-class-level-loading
branch
from
8786e61
to
f0c7fd1
Compare
When doing a
self.class.seedfrom within a test, we'd be within the Test Class scope, so the@loadervariable would assign there and and not onOaken::Seeds.loader.The old structure was a little muddy anyway, so this attempts a rewrite that's hopefully clearer.
Now we'll always be within
Oaken::Seeds'singleton_class.Note: originally the point about injecting
load_onto Oaken::Seedswas so you could potentially have other seeds modules, so you could have your app seeds split up ala Packwerk. But it doesn't really work and it's kinda unclear how I'd make it work, so it's probably better to yank it in a bit.cc @tcannonfodder