On 25/03/13 14:28, H.Merijn Brand wrote:
On Mon, 25 Mar 2013 13:55:26 +0000, "Martin J. Evans"
<boh...@ntlworld.com> wrote:
On 25/03/13 11:51, Tim Bunce wrote:
On Sun, Mar 24, 2013 at 09:08:33PM +0100, H.Merijn Brand wrote:
PASSes on all my boxes but one
The failing box has JSON::XS installed (temporary to check Schmorp's
claims, for which I so far found no proof)
What are those claims - is he claiming Perl has broken JSON::XS by any chance?
If you have (a lot of) time, read
https://rt.cpan.org/Public/Bug/Display.html?id=42462
and
https://rt.perl.org/rt3//Ticket/Display.html?id=117239
I don't have /that/ much time.
The thing(s) I wanted to check are if the module is now 100% C89 safe
(it is not) and if his claim that
#if defined(__BORLANDC__) || defined(_MSC_VER) || defined(__STDC__)
HE **hes = Perl_malloc (count * sizeof (HE *));
#else
HE *hes [count];
#endif
will slowdown the code significantly on systems that do support C99
nc
The formatted output - I might have an output patch applied to make the
output style meet our requirements - causes this mismatch.
Am I right in thinking that the goal of the JSON::XS test is to check
whether JSON::XS puts quotes around values?
It was a check to make sure that JSON::XS saw numbers in Perl as numbers
and not strings and so does not put quotes around them - the
DiscardString bit.
I'd suggest adding s/\s+//g, plus a comment, to make that explicit.
Why are there extra spaces in the result - there are not when I run it.
Because it is installed with a patch on this box to conform to the style
we require. Obviously this is an old and incomplete patch, that I never
bothered to fix as this module is prohibited in our company.
What I was pointing at, is that I would most like not be the only one
in the world to patch modules to behave as required, as not all modules
have decent ways for configurating needs and - as this is open source -
patching is allowed :)
People who change their modules to work differently from the ones on CPAN
surely need to accept that their change might break other things. The test in
question can work around it this time (because you've told us how you changed
it) but what about when you make another change to modify the JSON::XS module.
It seems like the test fails on this box because you changed JSON::XS and you
want the DBI test suite to keep up with you.
Having said that white space is allowed in JSON but you might find yourself on
a slippery slope as white space in quotes needs to be maintained.
If, as both Tim and me already assumed, JSON::XS is just used to check
quoting, s/\s+//g is a valid and safe fix.
Something else seems horribly broken here - admittedly I've not been
keeping up.
broken? IMHO JSON::XS is very very broken when ran on anything other
than GNU gcc supported platforms.
Merijn, I know your feelings on Marc Lehmann but I don't see they have any relevance
here. Whether you like it or not, JSON::XS is the fastest JSON parser module for Perl
that I am aware of (by all means correct me if you know better), probably the most
featureful and one a lot of people (including myself) depend on. The test in question was
added to check the changes I made for DiscardString and StrictlyTyped which I TOTALLY
rely on as we are producing massive JSON files and every extra "" makes a big
difference.
Martin
I don't mind when the verdict is: we cannot expect people to alter
whitespacing in module output, but having cpanprefs not only makes that
easy, but make installing a lot of modules suddenly become more logical
as I can "fix" the decisions the author made that I do not agree with.
Personally I don't think prettied output checks in tests should ever
test on whitespace (including newlines) unless the output is completely
generated by the module itself or guaranteed by the module or its
documentation to not add or remove whitespace.
The change to prevent this specific case is maybe somthing like
--8<---
--- a/t/90sql_type_cast.t 2013-03-24 21:00:02.167352360 +0100
+++ b/t/90sql_type_cast.t 2013-03-24 21:05:07.251376420 +0100
@@ -116,7 +116,7 @@ foreach my $test(@tests) {
skip 'DiscardString not supported in PurePerl', 1
if $pp && ($test->[3] & DBIstcf_DISCARD_STRING);
- my $json = JSON::XS->new->encode([$val]);
+ (my $json = JSON::XS->new->encode([$val])) =~ s/\s+]$/]/;;
#diag(neat($val), ",", $json);
is($json, $test->[5], "json $test->[0]");
};
-->8---
but it doesn't catch changes that generate extra spaces output as
like " [ 99 ] "
For me, the tests will pass again next week, as JSON::XS will be banned
to trash again
t/87gofer_cache.t ............... ok
t/90sql_type_cast.t ............. 1/45
# Failed test 'json undef'
# at t/90sql_type_cast.t line 121.
# got: '[null ]'
# expected: '[null]'
# Failed test 'json invalid sql type'
# at t/90sql_type_cast.t line 121.
# got: '["99" ]'
# expected: '["99"]'
# Failed test 'json non numeric cast to int'
# at t/90sql_type_cast.t line 121.
# got: '["aa" ]'
# expected: '["aa"]'
# Failed test 'json non numeric cast to int (strict)'
# at t/90sql_type_cast.t line 121.
# got: '["aa" ]'
# expected: '["aa"]'
# Failed test 'json small int cast to int'
# at t/90sql_type_cast.t line 121.
# got: '["99" ]'
# expected: '["99"]'
# Failed test 'json 2 byte max signed int cast to int'
# at t/90sql_type_cast.t line 121.
# got: '["32767" ]'
# expected: '["32767"]'
# Failed test 'json 2 byte max unsigned int cast to int'
# at t/90sql_type_cast.t line 121.
# got: '["65535" ]'
