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. 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 the author or his behavior). > 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 -- 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/
