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

Remove JSON::Parser as a dependency for the test suite #148

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

hakonhagland
Copy link
Contributor

JSON::Parser is no longer available on CPAN. It was removed from the JSON module in 2010, see this Changelog. Including it as a dependency for the test suite can cause the travis test to install the JSON module version 1.15 from 2007 (which includes JSON::Parser as a sub module). Incidentally this also installs an old version of JSON::PP (also included in the 2007 version of the JSON module) which can prevent the installation of a newer version of JSON::PP which again can cause test failures (see e.g. test t/104-filter_web.t) since the tests expect a newer version of JSON::PP to be installed.

I am curious if this patch will solve the failed test in PR #147 or if there is something else going on.

JSON::Parser is no longer available on CPAN. It was removed from the
JSON module in 2010. Including it as a dependency can cause the travis
test to install the JSON module version 1.15 from 2007 (which includes
JSON::Parser as a sub module). Incidentally this also installs an old
version of JSON::PP (also included in the 2007 JSON module) which can
prevent the installation of a newer version of JSON::PP which again
can cause test failures (see e.g. test t/104-filter_web.t).
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 88.814% when pulling b3c9bcc on hakonhagland:json_parser into 3e87a4d on garu:master.

@garu
Copy link
Owner

garu commented Aug 14, 2020

Hi @hakonhagland! Thanks for the help trying to understand what is going on with that test failure, and for all the work you've been putting on the #74 PR (which I really hope gets merged soon).

Because Data::Printer is meant to help debug code, I feel a bit torn about removing support for a module, even if it's not on CPAN anymore. However, we do have tests for JSON::PP so I don't understand why we had separate tests for JSON::Parser if it was a submodule. Moreover, I don't understand why it would grok on a simple JSON string 😭

But I also feel like mining through BackPAN to try and figure out an error that's not even coming from Data::Printer, but from a filter that will (likely) rarely be used might be just too much work. So let's just do this and see what happens :)

Thanks again!

@garu garu merged commit 269a5a3 into garu:master Aug 14, 2020
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.

3 participants