search

zendframework / zend-servicemanager

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

Improved tests aliases with abstract factories. #241

Closed jaapio closed 6 years ago

jaapio commented 6 years ago

There was an error in the testcase testAbstractFactoryShouldBeCheckedForResolvedAliasesInsteadOfAliasName where the returnValueMap of phpunit was used wrong. This caused a false positive. The test was returning an array which is interpreted as TRUE.

I discovered this when I added the testResolvedAliasFromAbstractFactory, which should do the opposite form the earlier mentioned testcase.

Beside that I found a way that the servicemanager was returning null instead of returning false when the last fallback on abstract factories failed.

I'm not sure if the testAbstractFactoryShouldBeCheckedForResolvedAliasesInsteadOfAliasName is still doing what was intended to do. So that might be a point of discussion.

fhein commented 6 years ago

You are right. It was me ‚fixed‘ this test and left it broken with #221. I am sorry.

The issue is fixed in #231.

Ocramius commented 6 years ago

@fhein btw, no need to be sorry - the original patch was legitimate, we simply missed it 👍

Ocramius commented 6 years ago

Manually merged via 54f5a52d113adaa33222bfcbf807e2b001d5d94c

jaapio commented 6 years ago

Cool, thanks for merging

fhein commented 6 years ago

:+1: . I'am somewhat embarrased to have introduced this hack without knowing what I did. :)