These are chat archives for dry-rb/chat

11th
Nov 2015
Piotr Solnica
@solnic
Nov 11 2015 13:47
@AMHOL wdyt about a feature in dry-container to memoize resolved object?
container.register(‘foo’, singleton: true) { Foo.new }
would that be too much?
I think it’s gonna be a common pattern that you only want to have one instance
Andy Holland
@AMHOL
Nov 11 2015 13:54
@solnic just do container.register('foo', Foo.new)
Piotr Solnica
@solnic
Nov 11 2015 13:55
I can’t in some places
becauase Foo.new may need Bar.new
Andy Holland
@AMHOL
Nov 11 2015 13:55
If Foo#call is implemented, you'll need container.register('foo', Foo.new, call: false)
Fair point
Yep, I think we could add that interface
Piotr Solnica
@solnic
Nov 11 2015 14:01
that would be awesome
it would give a speed boost
ie my current rails app has everything provided by the container, on each request we build a lot of objects and many of them can be singletons
like rom repos etc
Andy Holland
@AMHOL
Nov 11 2015 14:03
Yeah, makes sense, I didn't think about that when I made the register('blah', Blah.new) interface, I think I might remove that
Piotr Solnica
@solnic
Nov 11 2015 14:03
it shouldn’t be too hard to do on the container side and it would simplify things
Andy Holland
@AMHOL
Nov 11 2015 14:03
Could probably simplify things if I remove that and the proc interface and only allow block registration with options
Piotr Solnica
@solnic
Nov 11 2015 14:04
yes
Piotr Solnica
@solnic
Nov 11 2015 14:20
@AMHOL it would also allow us to require files within the block
and it would be called just once
I basically want a way of having complete isolation wrt deps
Andy Holland
@AMHOL
Nov 11 2015 14:21
Yeah, although that feels a bit wrong, dunno why lol
Piotr Solnica
@solnic
Nov 11 2015 14:21
so ie I want to write a test, require only the container, and when I ask for a specific dep I know only min will be required and instantiated
I dunno, I had the same feeling initially, but now if kinda feels right
needs further thinking I suppose
Andy Holland
@AMHOL
Nov 11 2015 14:24
Yeah, makes sense I guess
I just think it's better to require everything eagerly during setup
Piotr Solnica
@solnic
Nov 11 2015 14:24
worth to experiment with this
when booting, YES OF COURSE
when loading a test, NO OF COURSE NOT
ie rspec spec/unit/foo_spec.rb should run in complete isolation
wrt deps
atm I require spec-helper which loads the app, this is nonsense
when I see “123421.214412341 seconds to load files” in the rspec output, I cry
Andy Holland
@AMHOL
Nov 11 2015 14:42
lmao
Fair point
If you load in isolation, how do you know the method under test hasn't been monkey wrenched though?
Piotr Solnica
@solnic
Nov 11 2015 14:44
wdym?
Andy Holland
@AMHOL
Nov 11 2015 14:47
Forgot the :trollface:
lol
Monkey patched
Piotr Solnica
@solnic
Nov 11 2015 14:47
sorry, didn’t get the joke, FUNNY :D
or scary, not sure
Andy Holland
@AMHOL
Nov 11 2015 14:47
Yeah
It's funny because it's true
Piotr Solnica
@solnic
Nov 11 2015 14:48
right :)
I can now see what changes are needed to achieve isolation
I need to separate container configuration from finalization
it was collapsed into one thing, it is a mistake
Andy Holland
@AMHOL
Nov 11 2015 14:49
Makes sense, better to be explicit too
Piotr Solnica
@solnic
Nov 11 2015 14:49
then, I need to make a smarter import function, that would require missing deps
which should be trivial
pretty much looping through names, asking container if it exists, if not, gsubbing dots with slashes, asking container to require missing stuff
I want to require the container in the spec helper and it should have the config in place, bkz config is a dependency too for stuff like db url etc
then the import stuff should be able to take care of everything
ultimate goal is to be able to run tests in complete isolation
so you don’t load tons of files w/o any good reason
I will call this feature “Summer"
becauase summer comes after spring
Andy Holland
@AMHOL
Nov 11 2015 14:52
:joy:
Piotr Solnica
@solnic
Nov 11 2015 19:48
sweet
can we have include Dry::Constructor(:foo, :bar, kwargs: true) too? :D
although I gotta say my use-case is pretty test-specific
so ie I want to do Something.new(foo: double) and don’t care about the rest
with positioned args it is weird, I gotta pass nils
nils are replaced by auto-inject
this needs some thinking
Andy Holland
@AMHOL
Nov 11 2015 19:50
Ahh OK
Piotr Solnica
@solnic
Nov 11 2015 19:50
I still don’t have a way of using container to provide mocked deps
Andy Holland
@AMHOL
Nov 11 2015 19:51
Well to provide mocked deps, skip the container registration step, in test env and register your mocks instead
Piotr Solnica
@solnic
Nov 11 2015 19:52
that’s not that simple
Andy Holland
@AMHOL
Nov 11 2015 19:52
How come?
Piotr Solnica
@solnic
Nov 11 2015 19:52
when running all tests I don’t want to have mocked deps globally
I only want mocked deps for specific tests
we gotta solve this part in some smart way
Andy Holland
@AMHOL
Nov 11 2015 19:53
Could add a test mode
Piotr Solnica
@solnic
Nov 11 2015 19:54
how would that work?
Andy Holland
@AMHOL
Nov 11 2015 19:56
Was thinking rather than https://github.com/dryrb/dry-container/blob/master/lib/dry/container/registry.rb it would not raise on registration of a duplicate key
Piotr Solnica
@solnic
Nov 11 2015 19:56
a simple way would be sth like container.stub(‘persistence.foo’ => double())
Andy Holland
@AMHOL
Nov 11 2015 19:56
But that would overwrite the existing value
Piotr Solnica
@solnic
Nov 11 2015 19:56
of course only in test mode
Andy Holland
@AMHOL
Nov 11 2015 19:56
Yeah, but it could be done via configuration
Piotr Solnica
@solnic
Nov 11 2015 19:56
it would have to remember original value and restore it on teardown
this would be super useful
Andy Holland
@AMHOL
Nov 11 2015 19:58
Yeah, still not sure of the best way to go about it
Piotr Solnica
@solnic
Nov 11 2015 19:59
we won’t know until we try
I can tell you I have use cases for local-per-test stubs and global
global in the sense that I have a dep that I want to stub completely for all tests
it’s my thing, there are separate tests for it so I’m safe, but for all the tests that use objects which rely on it, I want to use a stubbed version
Andy Holland
@AMHOL
Nov 11 2015 20:11
Create something like dry-container-rspec or something?
Piotr Solnica
@solnic
Nov 11 2015 20:12
no, this doesn’t feel needed
it could be an internal thing, non-test-framework-specific
Andy Holland
@AMHOL
Nov 11 2015 20:15
Feels weird to update the actual library just for testing though
Piotr Solnica
@solnic
Nov 11 2015 20:15
I dunno
no super strong opinions
otoh it feels essential to have this kind of an interface for tests
Andy Holland
@AMHOL
Nov 11 2015 20:17

You happy with

Dry::Container.configure do |config|
  config.registry = Dry::Container::Test::Registry.new
end

For the things you want to stub for all tests?

Piotr Solnica
@solnic
Nov 11 2015 20:18
if we could collapse that into Dry::Container.test_mode! then yes, I guess, need to think about this :)
Andy Holland
@AMHOL
Nov 11 2015 20:21
Yeah