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

Fixing tests for bugfixed PHP versions #375

Merged
merged 6 commits into from
Jan 6, 2015
Merged

Fixing tests for bugfixed PHP versions #375

merged 6 commits into from
Jan 6, 2015

Conversation

urakozz
Copy link
Contributor

@urakozz urakozz commented Dec 25, 2014

Travis-CI tests fix (Final part of #292)

@AlmazDelDiablo
Copy link

+1

@urakozz
Copy link
Contributor Author

urakozz commented Jan 2, 2015

@stof could you please review this PR to close #292 at last

@stof
Copy link
Contributor

stof commented Jan 2, 2015

@urakozz I'm not a collaborator on this repo. So while I can still review this, I cannot merge it to fix the issue.

@urakozz
Copy link
Contributor Author

urakozz commented Jan 2, 2015

@stof I see)) But maybe your conclusion might attract attention to this PR =)

@schmittjoh
Copy link
Owner

I had a quick look. It seems like this is again fixing the tests instead of fixing the code. If I missed something, please let me know.

@urakozz
Copy link
Contributor Author

urakozz commented Jan 6, 2015

@schmittjoh Since DOMDocument bug [https://bugs.php.net/bug.php?id=67081] was fixed, \DOMDocumentType->internalSubset returns null. I have provided hack, that allows us to retrieve this subset anyway, please take a look.

*/
private function getDomDocumentTypeEntitySubset(\DOMDocumentType $child, $data)
{
if(!$this->isBugFixedPhpVersion()){
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put the version check here directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

*/
private function getDomDocumentTypeEntitySubset(\DOMDocumentType $child, $data)
{
if((PHP_VERSION_ID >= 50513) || (PHP_VERSION_ID >= 50429 && PHP_VERSION_ID < 50500)){
Copy link
Contributor

Choose a reason for hiding this comment

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

aren't you missing a negation here ?

Copy link
Owner

Choose a reason for hiding this comment

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

Instead of checking for all the versions here, can we check for whether $child->internalSubset contains the data that we expect? That seems more robust to me.

schmittjoh added a commit that referenced this pull request Jan 6, 2015
Fixing tests for bugfixed PHP versions
@schmittjoh schmittjoh merged commit 43bdac4 into schmittjoh:master Jan 6, 2015
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.

4 participants