search

zendframework / zend-test

Test component from Zend Framework
BSD 3-Clause "New" or "Revised" License
18 stars 38 forks source link

Provid a mechanism to check if we need to restore global var #73

Closed fezfez closed 5 years ago

fezfez commented 5 years ago

Hello,

Here is the patch to prevent #72 issue.

fezfez commented 5 years ago

@Ocramius : The Unit tests is not working as expected, i check that

Ocramius commented 5 years ago

One note here: I think the original mistake that causes https://github.com/zendframework/zend-test/issues/72 and https://github.com/zendframework/zend-test/issues/71 is coming from https://github.com/zendframework/zend-test/pull/66. One of the ideas behind the design of ZF2 (and now newer versions) was to never modify the operating environment of downstream consumers, and we've broken that.

@webimpress can you maybe check this as well? There should be a different solution that can replace #66.

michalbundyra commented 5 years ago

@Ocramius Sure, I'll have on it today afternoon. I see clearly there is some issue, but I have completely forgotten why I have done that fix...

fezfez commented 5 years ago

@Ocramius : test now work as expected and the attribute is now protected

michalbundyra commented 5 years ago

@Ocramius The easiest way is to xdebug tests I've added in #66. The solution I've applied is copied from zend-http, we are doing it there in several tests. The "source problem" is here: https://github.com/zendframework/zend-http/blob/bdfe6ca648829c681c20323f49071fbe645caa7e/src/PhpEnvironment/Request.php#L476-L542 where we are using values from $_SERVER to determine the path/base url. And as phpunit is a runner, this is why we had problem with this value in a request path.

Thoughts?

Ocramius commented 5 years ago

What we could probably do is provide a utility to back up/restore values, rather than resetting them upfront.

michalbundyra commented 5 years ago

Hm... How do you mean? You want someone manually "reset them" if they are using "phpunit" parameter in the query/path? I was checking if we can do it somehow on dispatch, but I can't see any clear and nice solution right now.

As I noted in my PR, I wasn't sure if it is the best solution and what impact it's gonna have. It works for us, but as we see, not for everyone...

weierophinney commented 5 years ago

Closing in favor of #74.