Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Prevent unit tests from fatally failing if database is not available. #1454

Merged
merged 2 commits into from Sep 10, 2012

Conversation

aaronschmitz
Copy link
Contributor

Without these changes, all unit test crash when phpunit attempts to call a method on a null object.

@stefanneculai
Copy link
Contributor

Take a look at the following line:
https://github.com/aaronschmitz/joomla-platform/blob/50c7794826a7e763604f37e6c07412564f961188/tests/core/case/database.php#L491

I think this line should mark the test as incomplete if driver=null and this way getConnection is never called. I would say that there is a problem with using empty which has a strange behavior depending of PHP version. Try is_null(static::$driver) instead of empty(static::$driver) - this may fix the issue.

This is only a guess. Sorry if I am wrong.

@aaronschmitz
Copy link
Contributor Author

I don't follow how that change will prevent getConnection from being executed - please explain.

@stefanneculai
Copy link
Contributor

It doesn't look like PHPUnit_Extensions_Database_TestCase uses getConnection before setUp so I supposed that it is called after that. If driver is null then the test should stop and be marked as incomplete. This way getConnection is never called if driver is null. Maybe I'm wrong.

@aaronschmitz
Copy link
Contributor Author

I hope this is along the lines of what you were thinking, @stefanneculai.

@stefanneculai
Copy link
Contributor

I was suggesting that the fix to this issue would be just to change if (empty(static::$driver)) from http://goo.gl/p3spv to if (is_null(static::$driver)) or if (static::$driver === null). I think this could replace all the changes that you did in this pull request. Maybe you can give it a try.

@aaronschmitz
Copy link
Contributor Author

According to the PHP documentation empty(null) == true - http://php.net/manual/en/function.empty.php. The problem is that marking the unit test as skipped doesn't immediately terminate the current test.

@stefanneculai
Copy link
Contributor

I know that but on some versions of PHP empty(null) returns false. :-)

Also, marking a test as skipped terminates the current test immediately (I have just tried that).

@aaronschmitz
Copy link
Contributor Author

Well, I tried making the change you suggested on 491 just to be sure, and it didn't work. If you have or anyone else have a simpler solution that solves the problem, I'm all ears... but I haven't found one yet.

@LouisLandry
Copy link
Contributor

This looks good enough. Thanks.

LouisLandry added a commit that referenced this pull request Sep 10, 2012
Prevent unit tests from fatally failing if database is not available.
@LouisLandry LouisLandry merged commit fcf43ef into joomla:staging Sep 10, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants