search

zendframework / zend-pimple-config

Pimple container configurator based on ZF ServiceManager configuration
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

Configuration consistency fixes #10

Closed michalbundyra closed 6 years ago

michalbundyra commented 6 years ago

It should be compatible with https://github.com/weierophinney/zend-container-config-test and docs https://docs.zendframework.com/zend-expressive/v3/features/container/config/

The main issue is that container passed to the factory is not the same instance as the original container. This problem is because we configure pimple container (non-psr-11) and we wrap it with PSR-11 container when we need and when we pass it into the factories: https://github.com/webimpress/zend-pimple-config/blob/hotfix/config-consistency/src/Config.php#L93

This PR is based on #6

weierophinney commented 6 years ago

I suggest overriding testFactoryIsProvidedContainerAndServiceNameAsArguments within the test class in order to change the assertions around the container.

Alternately, we can simply check that the property is a PSR-11 container, as there's little question how that property is set.

michalbundyra commented 6 years ago

I think I would prefer the 2nd option, if we override test here it will be hacky solution. Maybe we should then add final for all our test cases in zend-container-config-test? If someone what have library compatible with our test it must pass all our test without hacking ;)

weierophinney commented 6 years ago

I think I would prefer the 2nd option, if we override test here it will be hacky solution.

Okay, those changes are made in the testing repository.

weierophinney commented 6 years ago

I've updated your branch to pin to the stable 0.1.0 release of zend-container-config-test, and updated the shipped test as well to extend AbstractExpressiveContainerConfigTest.

weierophinney commented 6 years ago

Thanks, @webimpress!