Re: 1.524
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)
>
> 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?
I'd suggest adding s/\s+//g, plus a comment, to make that explicit.
Tim.
> 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/
Re: 1.524
On Mon, 25 Mar 2013 11:51:09 +, 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)
> >
> > 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?
Note that up to now I never had JSON::XS installed anywhere, so I never
ran that test.
This file is completely mje's work (entered 2009-11-27 18:03:49)
git-svn-id: http://svn.perl.org/modules/dbi/trunk@13615
50811bd7-b8ce-0310-adc1-d9db26280581
Commit messages since
Tests for DBI::sql_type_cast - not necessarily finished but wanted to let
Tim see them.
Fix test count and tidy output for a full make test - disable warnings when
we expect them, remove diag calls
Needs SQL types and DBIstcf_XXX
Fix tests for conversions to IV on machines where IVs are 8 bytes
was testing longsize and it should have been ivsize
Added DBIstcf_STRICT and DBIstcf_DISCARD_STRING to DBI::PurePerl.
Added rough draft of sql_type_cast to DBI::PurePerl.
Fixed typo in Changes.
Removed DBI:: prefixes from t/90sql_type_cast.t
(Some tests still fail)
Some tests cannot be performed with PurePerl e.g., DiscardString (skipped)
and number overflows do not work the same (don't overflow).
Omit some tests on Perl < 5.10.1 due to a change in sv_2nv.
Fixed "Argument "aa" isn't numeric in addition" warnings from
t/zvp_90sql_type_cast.t
Only warn in PurePerl sql_type_cast if warnings are enabled.
> I'd suggest adding s/\s+//g, plus a comment, to make that explicit.
I think I agree, but I'd like to hear Martin's voice
> Tim.
>
> > 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/
Re: 1.524
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?
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.
Something else seems horribly broken here - admittedly I've not been keeping up.
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/
Re: 1.524
On Mon, 25 Mar 2013 13:55:26 +, "Martin J. Evans"
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
Re: 1.524
On 25/03/13 14:28, H.Merijn Brand wrote:
On Mon, 25 Mar 2013 13:55:26 +, "Martin J. Evans"
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
Re: 1.524
On 25 March 2013 15:48, Martin J. Evans wrote: > as we are producing massive JSON files and every extra "" makes a big > difference. If there is no specific reason to use JSON (like for instance sending data to a web browser) and size and speed are priorities then you might want to consider using Sereal instead of JSON. It is both faster and produces smaller output than JSON. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
Re: 1.524
On Mon, 25 Mar 2013 14:48:07 +, "Martin J. Evans" wrote: > On 25/03/13 14:28, H.Merijn Brand wrote: > > On Mon, 25 Mar 2013 13:55:26 +, "Martin J. Evans" > > 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 I will try to find out on a real-world example > 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. I do > 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. I already have sent patches to other CPAN authors where tests failed on *formatted* output because the module used changed its unpatched defaults. What I learned is that one should write tests that are not depending on the layout *formatted* output from third parties give. JSON::XS is just an example here, but the same happens with HTML and xml, where a lot of whitespace is not important for the final output. What I am aiming at here is that you test what you want to test: quotation. The use of quotation is documented. As a counterexample: I had to file patches for perltidy, as they did not take into consideration that someone might test perltidy with a ~/.perltidyrc already present. Even the smallest change would break the complete test suite. This can happen to *any* module that has external influences on the produced output. > 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. yes and no: I want tests to test what they want to test, not depend on side effects. That is why I just test on error NUMBERS and not on the error string when the string can be locale depending. > 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. Why not use JSON::PP? It is CORE and the test has *tiny* structures to test, so speed is irrelevant. > > 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. Correct. They don't > 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), No, you are correct. But that also implies that you are able to install it in the first place, and up till today, it will take me *considerable* time, to have it compile on some of the compilers I am forced to use. Like it or not. The fact that it is faster than any other module does not make up for that. (and it also has nothing to do with th
Re: 1.524
On 25/03/13 14:53, demerphq wrote: On 25 March 2013 15:48, Martin J. Evans wrote: as we are producing massive JSON files and every extra "" makes a big difference. If there is no specific reason to use JSON (like for instance sending data to a web browser) and size and speed are priorities then you might want to consider using Sereal instead of JSON. It is both faster and produces smaller output than JSON. Yves Thanks Yves. I've tried Sereal but unfortunately it is just too late to change now. Other reasons are a) it is not readable b) last time I looked it was not supported by all the languages we need to support c) it is still marked experimental. Martin
Re: 1.524
On 25/03/13 15:12, H.Merijn Brand wrote: On Mon, 25 Mar 2013 14:48:07 +, "Martin J. Evans" wrote: On 25/03/13 14:28, H.Merijn Brand wrote: On Mon, 25 Mar 2013 13:55:26 +, "Martin J. Evans" 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 I will try to find out on a real-world example 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. I do 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. I already have sent patches to other CPAN authors where tests failed on *formatted* output because the module used changed its unpatched defaults. What I learned is that one should write tests that are not depending on the layout *formatted* output from third parties give. JSON::XS is just an example here, but the same happens with HTML and xml, where a lot of whitespace is not important for the final output. If you can reliably take out the white space between elements of the JSON string without changing the meaning then by all means do so. What I am aiming at here is that you test what you want to test: quotation. The use of quotation is documented. As a counterexample: I had to file patches for perltidy, as they did not take into consideration that someone might test perltidy with a ~/.perltidyrc already present. Even the smallest change would break the complete test suite. This can happen to *any* module that has external influences on the produced output. 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. yes and no: I want tests to test what they want to test, not depend on side effects. That is why I just test on error NUMBERS and not on the error string when the string can be locale depending. 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. Why not use JSON::PP? It is CORE and the test has *tiny* structures to test, so speed is irrelevant. When I last looked other JSON parsers did not output JSON like JSON::XS. JSON::XS looks at a scalar's pv and if it does not exist outputs it as a number e.g., 1 and not "1". 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. Correct. They don't 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), No, you are correct. But that also implies that you are able to install it in the first place, and up till today, it will take me *considerable* time, to have it compile on some of the compilers I am forced to use. Like it or not. The fact that
Re: 1.524
On Mon, Mar 25, 2013 at 04:54:42PM +, Martin J. Evans wrote: > > The test only applies if a) you have JSON::XS installed and b) you > want to test DiscardString with JSON::XS (see above for reason why). > By all means take the test out if you like, I only included it because > it was my specific usage case which led to DiscardString and > StrictlyTyped. Which is important and worth keeping, I think. Failing due to a locally patched module is a local problem. Tim.
Re: 1.524
On Mon, 25 Mar 2013 17:24:41 +, Tim Bunce wrote: > On Mon, Mar 25, 2013 at 04:54:42PM +, Martin J. Evans wrote: > > > > The test only applies if a) you have JSON::XS installed and b) you > > want to test DiscardString with JSON::XS (see above for reason why). > > By all means take the test out if you like, I only included it because > > it was my specific usage case which led to DiscardString and > > StrictlyTyped. > > Which is important and worth keeping, I think. I never disagreed with that > Failing due to a locally patched module is a local problem. But easily caught. I pushed the change. IMHO no need for a ChangeLog entry though. > Tim. -- 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/
Re: 1.524
On 25 March 2013 17:45, Martin J. Evans wrote: > On 25/03/13 14:53, demerphq wrote: >> >> On 25 March 2013 15:48, Martin J. Evans wrote: >>> >>> as we are producing massive JSON files and every extra "" makes a big >>> difference. >> >> >> If there is no specific reason to use JSON (like for instance sending >> data to a web browser) and size and speed are priorities then you >> might want to consider using Sereal instead of JSON. It is both faster >> and produces smaller output than JSON. >> >> Yves > > > Thanks Yves. I've tried Sereal but unfortunately it is just too late to > change now. Other reasons are a) it is not readable b) last time I looked it > was not supported by all the languages we need to support c) it is still > marked experimental. Re a-c... A) its not readable, and it wont ever be. OTOH, would we be able to mitigate this factor for you if there was an easy to use to tool which would do something like translate a Sereal dump on the fly to something readable? You arent the first who has said that, and I can sympathize (although i have gotten surprisingly good at decoding sereal by sight :-), so i am wondering if there is a middle ground on this one that might tip the balance... (assuming the other points werent an issue). B) our cross language support is growing but still pretty small. By that standard I can understand sticking to JSON. OTOH, id be curious to know which languages are important to you. C) the experimental marking these days is mostly there so people give us feedback when they use it. We should probably change that advisory. (TBH, until you mentioned it I had forgotten it entirely.) Anyway, thanks for the feedback! Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
