Wikimedia Bugzilla is closed!

PHPUnit 3.7.8 by Sebastian Bergmann.

"sdfsedtgsrdysftu" seems to come from PHPUnit.

This is not content handler but assigning it to our team.

All tests pass for me. Can you provide more details on how to reproduce?

I'm just running make safe. Like I said above it probably has something to do with the PHPUnit version.

WFM is not very friendly solution when I can consistently reproduce it.

Reopen (perhaps move to another component if it's not contenthandler/if wikidata team doesn't want to fix it?).

What's this?

core/tests/phpunit/includes/libs/GenericArrayObjectTest.php:          try { $list->offsetUnset( 'sdfsedtgsrdysftu' ); } catch ( \Exception $exception ){}

(In reply to comment #5)
> WFM is not very friendly solution when I can consistently reproduce it.

Sorry, from what you said, I could only infer that it was an issue with phpunit. Nobody except you seems to be reproducing it. And you said '"sdfsedtgsrdysftu" seems to come from PHPUnit.'

(In reply to comment #6)
> Reopen (perhaps move to another component if it's not contenthandler/if
> wikidata team doesn't want to fix it?).

So... what you find below is a core test. So it should be filed as a core bug. Though having the Wikidata team at least in CC makes sense, since I suspect that Jeroen wrote this.
 
> core/tests/phpunit/includes/libs/GenericArrayObjectTest.php:          try {
> $list->offsetUnset( 'sdfsedtgsrdysftu' ); } catch ( \Exception $exception ){}

Huh, nice find! Well - that should trigger an exception. And then catch it. No reason that it should trigger an internal error. But perhaps it does, depending on how PHP and phpunit are configured.

What really throws me is that problem is happening for *nobody* else. Having a stack trace would really help. Niklas, any reason you are not using xdebug?

> core/tests/phpunit/includes/libs/GenericArrayObjectTest.php:          try {
> $list->offsetUnset( 'sdfsedtgsrdysftu' ); } catch ( \Exception $exception ){}

Perfectly legit code to test if an exception that should be thrown is actually thrown. We have such code at multiple places in Wikibase and no one has a problem with it there either.

https://gerrit.wikimedia.org/r/#/c/37470/

(In reply to comment #8)
> > core/tests/phpunit/includes/libs/GenericArrayObjectTest.php:          try {
> > $list->offsetUnset( 'sdfsedtgsrdysftu' ); } catch ( \Exception $exception ){}
> 
> Perfectly legit code to test if an exception that should be thrown is
> actually
> thrown. We have such code at multiple places in Wikibase and no one has a
> problem with it there either.

The problem with that test is that it checks for *any* exception to be thrown. It doesn't check whether the exception is the right exception.

What you end up catching in this particular case is the exception for the warning "Undefined index:  sdfsedtgsrdysftu". Which stops the unit test there, so you never find out that PHP's documentation for ArrayObject::offsetGet is incorrect (at least in php 5.2.6 and 5.4.4), because the method actually returns null rather than false:

  $ php -r '$a=new ArrayObject; var_dump( @$a->offsetGet("foobar") );'
  NULL

so the implementation of offsetUnset in includes/site/SiteArray.php doesn't work right. Never mind that it should probably be using offsetExists instead of offsetGet anyway.

Although the biggest question here is why does Niklas's installation of phpunit not respect the convertErrorsToExceptions="true" setting (and related settings) in MediaWiki's suite.xml?

From Gerrit: Btw, I did more testing and it seems that it only fails if I test all of includes but not if I only test includes/libs. Seems to be yet another problem of test state leaking.

After some googling I found out http://osdir.com/ml/php.phpunit.devel/2008-03/msg00065.html and then after testing I found out that commenting out set_error_handler in includes filebackend/FSFileBackend.php prevents the fatal error from happening. My best guess now is that filebackend is not restoring the error handler properly in some cases.

And I ** hate PHPUnit. I have a suspect where the restoration fails, because I start seeing warnings in middle of test run. When I'm trying to use --testdox to see the test names I don't see the warnings!

So, I start seeing warnings in FileBackendTest::testStore test. The warnings do not show up in the standard if I only run filebackend tests (but the tests are still marked as failed.

When running all tests:
Warning: fopen(/www/w/images/lockdir/khsf3ntb7why6dhpaq76lvaw45uil3u.lock): failed to open stream: Permission denied in /www/dev.translatewiki.net/w/includes/filebackend/lockmanager/FSLockManager.php on line 123

When running filebackend only:
1) FileBackendTest::testStore with data set #0 (array('store', '/tmp/unittests_663c2ecf29f8-1.txt', 'mwstore://localtesting/unittest-cont1/e/fun/obj1.txt'), '/tmp/unittests_663c2ecf29f8-1.txt', 'mwstore://localtesting/unittest-cont1/e/fun/obj1.txt')
fopen(/www/w/images/lockdir/khsf3ntb7why6dhpaq76lvaw45uil3u.lock): failed to open stream: Permission denied

But then again all of this can be unrelated, because even if I skip FileRepo and FileBackend tests, I still get the fatal error mentioned in the bug description.

I think I know why commenting out set_error_handler seemed to work. It just unset error_handler set by someone else.

Also, the fatal seems to be triggered in some other test, possibly inside hook call, since commenting out set/restore_error_handler in Hooks.php allows me to run "make safe" successfully.

So it is SiteArrayTest::testUnset which is failing, but again if I only run tests in includes/site they are not failing.

I've tried to debug this for two hours now, I don't know what else to do.

> PHP's documentation for ArrayObject::offsetGet is
> incorrect (at least in php 5.2.6 and 5.4.4), because the method actually
> returns null rather than false:

Good catch. https://gerrit.wikimedia.org/r/#/c/37637/

> I've tried to debug this for two hours now, I don't know what else to do.

As you're saying, the test is broken. So why not merge my removal of the test?

Gerrit change I4ce1651d "Fix check to see if element is there already."  is merged.

Gerrit change I9d14fd87 "Remove broken test." is also merged.

Is this bug still reproducible?

Most likely fixed, but I can't verify immediately due to bug 42145. I'll reopen if it surfaces.