Re: 1.524

On Mon, 25 Mar 2013 13:55:26 +0000, "Martin J. Evans"
<[email protected]> 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

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

> >> 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 :)

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.

> > Tim.
> 
> 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"   ]'

-- 
H.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/
using perl5.00307 .. 5.17   porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/        http://www.test-smoke.org/
http://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/