Skip to content
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

Exceptions in __toString won't show up. Added a trigger error for the user to be able to advise #5

Closed
wants to merge 8 commits into from

Conversation

geraldcroes
Copy link
Member

__toString cannot throw an exception. Added a try / catch around the __toString implementation in case of exceptions in cleanDirectory (for example).

@geraldcroes
Copy link
Member Author

Renamed $e to $exception.
Added a test
Added a test on is_dir in cleanDestinationDirectory for it to not try to clean the directory if it doesn't exists
Added a test on that.

BUT, I am not able to run tests on my computer for coverage/html.php, here is the output :

php ./scripts/runner.php -d tests/units/classes/report/fields/runner/coverage/

atoum version 325 by Frédéric Hardy (/home/gcroes/atoum)
PHP path: /usr/bin/php5
PHP version:
=> PHP 5.3.2-1ubuntu4.9 with Suhosin-Patch (cli) (built: May 3 2011 00:43:34)
=> Copyright (c) 1997-2009 The PHP Group
=> Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
=> with Xdebug v2.1.2, Copyright (c) 2002-2011, by Derick Rethans
mageekguy\atoum\tests\units\report\fields\runner\coverage\html...
[SSSSSSSSSSFSSSSSe___________________________________________][17/17]
=> Test duration: 0.58 second.
=> Memory usage: 4.50 Mb.
Total test duration: 0.58 second.
Total test memory usage: 4.50 Mb.
Code coverage value: 38.52%
=> Class mageekguy\atoum\report\fields\runner\coverage\html: 33.89%
==> mageekguy\atoum\report\fields\runner\coverage\html::__toString(): 5.19%
==> mageekguy\atoum\report\fields\runner\coverage\html::getDestinationDirectoryIterator(): 0.00%
==> mageekguy\atoum\report\fields\runner\coverage\html::cleanDestinationDirectory(): 21.43%
Running duration: 2.48 seconds.
Failure (1 test, 17 methods, 1 failure, 1 error, 0 exception) !
There is 1 failure:
=> mageekguy\atoum\tests\units\report\fields\runner\coverage\html::testCleanDestinationDirectory():
In file /home/gcroes/atoum/tests/units/classes/report/fields/runner/coverage/html.php on line 294, mageekguy\atoum\asserters\adapter::once() failed: function unlink() is called 0 time instead of 1
There is 1 error:
=> mageekguy\atoum\tests\units\report\fields\runner\coverage\html::test__toString():
Error 255 in /home/gcroes/atoum/tests/units/classes/report/fields/runner/coverage/html.php on unknown line, generated by file /home/gcroes/atoum/classes/mock/generator.php(159) : eval()'d code on line 17:
Fatal error: Call to a member function control() on a non-object

Call Stack:
0.0921 326180 1. {main}() -:0
0.1050 1386188 2. mageekguy\atoum\runner->run() -:1
0.1116 1634548 3. mageekguy\atoum\test->run() /home/gcroes/atoum/classes/runner.php:335
0.1344 1632420 4. mageekguy\atoum\test->runTestMethod() /home/gcroes/atoum/classes/test.php:434
0.1346 1674232 5. mageekguy\atoum\tests\units\report\fields\runner\coverage\html->test__toString() /home/gcroes/atoum/classes/test.php:559
0.3684 3513832 6. mock\mageekguy\atoum\template\tag->__construct() /home/gcroes/atoum/tests/units/classes/report/fields/runner/coverage/html.php:468
0.3690 3515752 7. mock\mageekguy\atoum\template\tag->setMockController() /home/gcroes/atoum/classes/mock/generator.php(159) : eval()'d code:39

@mageekguy
Copy link
Contributor

I have added exception management.
Can you update your patch ?

@geraldcroes
Copy link
Member Author

If the directory is supposed to exists, then I think you should remove the is_dir()/mkdir() part (https://github.com/mageekguy/atoum/blob/master/classes/report/fields/runner/coverage/html.php#L302)

If not, then you should add the test to avoid the exception that will be raised when trying to clean the directory.

Your decision
(or I may have overlooked the given code....)

@mageekguy mageekguy closed this Aug 3, 2011
mikaelrandy pushed a commit that referenced this pull request Oct 20, 2016
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.

None yet

2 participants