Skip to content

test to function ErrorException::getSeverity(); #1770

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

Closed
wants to merge 4 commits into from

Conversation

marcosptf
Copy link
Contributor

was add a test to method uncovered from class ErrorException,and i test too anothers inheritance methods.

was add a test to method uncovered from class ErrorException,and i test too anothers inheritance methods.
@krakjoe
Copy link
Member

krakjoe commented Oct 17, 2016

This test is not clear enough, don't use magic numbers.

@marcosptf
Copy link
Contributor Author

Hello @krakjoe how are you?
fine?

please, give me some example and ill fix this code!
thanks for your comment!

@marcosptf
Copy link
Contributor Author

"This test is not clear enough, don't use magic numbers."

@krakjoe
can you explain please what is magic numbers and what i can do to fix this function like you suggest?

Thanks!

/* Trigger exception */
preg_match();
?>
--EXPECTF--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these values here should be predictable, such as message, severity, code, previous, file & line.

}
throw new ErrorException($message, $code, $severity);
} catch(ErrorException $err) {
var_dump(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this long var_dump(), you can also test for \Exception's magic __toString() method by calling:

echo $err;


$message = "error exception, severity =>";
$code = 500;
$severity = 75;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like @krakjoe mentioned, please do not use magic numbers, severity here should mimic the E_* constants, so you could for example assign $severity to E_WARNING and just give the $code a value of 0 since you are testing for getSeverity here anyway

@marcosptf
Copy link
Contributor Author

@krakjoe @KalleZ

okey i'll fix this test!

Thanks have a nice day!

@KalleZ
Copy link
Member

KalleZ commented Oct 26, 2016

Thanks, also remember you can define error_reporting in an --INI-- section, so that you can reliably determine that whatever value you set $severity to will not fail the: if(!(error_reporting() & $severity)) check, thanks for your work :)

@marcosptf
Copy link
Contributor Author

okey @KalleZ

Thank you for your explanation !!!

👍

@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

@marcosptf bump ... you forgot about this one :)

@marcosptf
Copy link
Contributor Author

im fixing it...

@marcosptf marcosptf changed the title test to function ErrorException::getSeverity(); WIP: test to function ErrorException::getSeverity(); Feb 8, 2017
@marcosptf
Copy link
Contributor Author

Hi @krakjoe @KalleZ

i would like to thanks for the comments and the explain, i really really appreciate it!!!

i've fixed like the comments, trying test most of possible cases.

thanks so much!!!!!

@marcosptf marcosptf changed the title WIP: test to function ErrorException::getSeverity(); test to function ErrorException::getSeverity(); May 17, 2017
@marcosptf
Copy link
Contributor Author

ping @krakjoe @KalleZ

@krakjoe
Copy link
Member

krakjoe commented Jun 1, 2017

Merged 3efef6d

Thanks.

@krakjoe krakjoe closed this Jun 1, 2017
@marcosptf
Copy link
Contributor Author

marcosptf commented Jun 1, 2017

again @krakjoe @KalleZ
thanks so much for your time and explanation to do this task
have a nice day!!!!

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

Successfully merging this pull request may close these issues.

4 participants