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

testParseErrorDetectSingleQuotes() fails on pecl-json-c #2458

Closed
wants to merge 1 commit into from

Conversation

donquixote
Copy link
Contributor

Adding a comment to explain why testParseErrorDetectSingleQuotes() fails on some PHP configurations.

I would also add a message to be printed when the test fails, but expectParseException does not currently accept a parameter. Too lazy to change that.. (and prefer to work on the PSR-4 stuff)

@donquixote
Copy link
Contributor Author

Meant to fix #2457

// The regular PHP json extension does not accept single quotes.
// However, some versions of pecl-json-c do accept them, causing this
// test to fail. This is fixed in
// https://github.com/remicollet/pecl-json-c/commit/c15ddc2a95fece22a65a05eeeb4bfdd63a745273
Copy link
Contributor

Choose a reason for hiding this comment

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

Linking to this commit is misleading because if you look at the current README, that PR has been merged upstream in json-c. So what we are actually waiting for is a release of json-c with that fix, and ubuntu/debian to ship that fix also.

@staabm
Copy link
Contributor

staabm commented Nov 26, 2013

what about using skipTest instead in case json-c extension is loaded? maybe with a additional if case for the json-c version..

@donquixote
Copy link
Contributor Author

@staabm: The next version of json-c could have this fixed already.
What we could do is feature detection on raw json_decode(), and then let the test behave accordingly.
The question is how much energy do we want to invest in this..

@Seldaek
Copy link
Member

Seldaek commented Nov 27, 2013

I'd tend towards the "not my bug, don't care" resolution for this :) Given it's a temporary issue that will hopefully be soon resolved I think it's safe to just ignore. If you want to do something about it though then feel free but then the json_decode test you suggested is probably a better idea, because just adding a comment still means people will waste time.

@Seldaek Seldaek closed this Nov 27, 2013
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

4 participants