On 25/03/13 15:12, H.Merijn Brand wrote:
On Mon, 25 Mar 2013 14:48:07 +0000, "Martin J. Evans"
<boh...@ntlworld.com> wrote:
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
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 it is faster than any
other module does not make up for that. (and it also has nothing to do
with the author or his behavior).
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.
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.
Not arguing against that at all
Martin
