search

zendframework / zend-servicemanager

ServiceManager component from Zend Framework
BSD 3-Clause "New" or "Revised" License
188 stars 89 forks source link

Adds tests performing permutations around all possible get/create sequences that may lead to inconsistent shared instances as per #236 #237

Closed fhein closed 6 years ago

fhein commented 6 years ago

Adds a test addressing the issue described in #236. 

/**
 * The ServiceManager can change internal state on calls to get,
 * build or has, latter not currently. Possible state changes
 * are caching a factory, registering a service produced by
 * a factory, resolving an alias and so on.
 *
 * This tests performs three consecutive calls to build/get for
 * each registered service to push the service manager through
 * all internal states, thereby verifying that build/get/has
 * remain stable through the internal states.
 */

The test does not make any assumptions about particular internal states the ServiceManager has or might have. The ServiceManager is treated as a blackbox in order to make sure, that the test is agnostic to possible future changes to the implementation of internal states.

fhein commented 6 years ago

Some older tests will become redundant if this will be accepted. I hesitate to cleanup such redundancies, because I didn't implement them and I don't want that people miss their tests.

I offer to care about those redundant tests and clean up, if you think that this is necessary.

Ocramius commented 6 years ago

Changed base branch to develop. If this needs backporting, we'll probably have to redo the patch via cherry-picking

Ocramius commented 6 years ago

Some older tests will become redundant if this will be accepted.

We should try removing them here after the comments above have been addressed/discussed, or else this will pass under the radar again. Also, that is a good chance to figure out whether the older tezts actually cover more "edgy" scenarios.

fhein commented 6 years ago

Isn't that idempotency thing a general issue? Wouldn't it be good to have all applicable tests wrapped into this one?

fhein commented 6 years ago

The current version of the test unreveals a possible bug: build can produce a shared service allthough sharedByDefault is false.

Could you please review to make sure, that it's not the test which is buggy. I should write a test for the test. :)

Forget it. The test does not consider that a registered shared service is returned shared in all cases.

Fixed.

fhein commented 6 years ago

Anything left for me to do about this?