Skip to content

Fix phpunit errors with php notice already defined #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix phpunit errors with php notice already defined #8

wants to merge 1 commit into from

Conversation

cepa
Copy link

@cepa cepa commented Jun 23, 2013

Hi,

this is a small fix for smartypants to avoid throwing php notice about a constant already defined in phpunit tests if smartypants is a part of a project.

thx

@michelf
Copy link
Owner

michelf commented Jun 23, 2013

I don't understand why this is needed. Do you import/require smartypants.php more than once? If not there shouldn't be any conflict.

@cepa
Copy link
Author

cepa commented Jun 23, 2013

Well, this should not be a problem in "production" code, but in phpunit with process isolation this file can be loaded multiple times and that throws php notice:

RuntimeException: PHP Notice:  Constant SMARTYPANTS_VERSION already defined in /home/test/workspace/php_commons-4.3/CONFIG/MYSQL51/label/test-php53/vendor/michelf/php-smartypants/smartypants.php on line 15
PHP Stack trace:
PHP   1. {main}() -:0
PHP   2. require_once() -:56
PHP   3. require_once() /home/test/workspace/php_commons-4.3/CONFIG/MYSQL51/label/test-php53/test/bootstrap.php:56
PHP   4. ComposerAutoloaderInit87a3dd12578d5de70d91fd42de9a6d6a::getLoader() /home/test/workspace/php_commons-4.3/CONFIG/MYSQL51/label/test-php53/vendor/autoload.php:7
PHP   5. require() /home/test/workspace/php_commons-4.3/CONFIG/MYSQL51/label/test-php53/vendor/composer/autoload_real.php:42
PHP   6. define() /home/test/workspace/php_commons-4.3/CONFIG/MYSQL51/label/test-php53/vendor/michelf/php-smartypants/smartypants.php:15

And this multiplied by the number of tests :)

The fix does not affect the use of smartypants but it helps to avoid this issue and it allows to override default constant values.

@michelf
Copy link
Owner

michelf commented Jun 23, 2013

Does that so called "process isolation" works by including things many times in different namespaces? If that's the case, it won't work with code that use define, because constants defined with define aren't isolated by namespace. You should be able to change those global defines with const CONSTANT = "value" instead to make isolation work. I'm not going to pull such a change however since it requires PHP 5.3.

That said, I do plan to make a new version of PHP Smartypants mimicking the new lib branch in PHP Markdown that will target PHP 5.3 and get rid of those define calls (among other things).

@cepa
Copy link
Author

cepa commented Jun 23, 2013

The thing with define() and phpunit is already reported: sebastianbergmann/phpunit#314
Anyways I will keep the fork of smartypants because the fix with defined() works fine and solves problem across php-5.x setups.
Can't wait to see the new version of smartypants :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants