Test failures from incorrect json string escaping

[Martin Packman]

As noted in _loader._MemObjectProxy.to_json the way values are stringified is not compatible with json encoding, resulting in the following test failures:

======================================================================
FAIL: test_to_json (meliae.tests.test_loader.TestMemObj)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "meliae\tests\test_loader.py", line 239, in test_to_json
    self.assertEqual(expected, [obj.to_json() for obj in objs])
AssertionError: Lists differ: ['{"address": 1, "type": "tupl... != ['{"address": 1, "type": "tupl...

First differing element 5:
{"address": 6, "type": "str", "size": 29, "value": "a str", "refs": []}
{"address": 6, "type": "str", "size": 29, "value": "a str"", "refs": []}

Diff is 720 characters long. Set self.maxDiff to None to see it.

======================================================================
FAIL: test_expand_refs_as_dict (meliae.tests.test_loader.TestObjManager)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "meliae\tests\test_loader.py", line 404, in test_expand_refs_as_dict
    self.assertEqual({1: 'c', 'b': 'c'}, as_dict)
AssertionError: {1: 'c', 'b': 'c'} != {1: 'c"', 'b"': 'c"'}
- {1: 'c', 'b': 'c'}
+ {1: 'c"', 'b"': 'c"'}
? + + +

It would be possible to use json.encoder.encode_basestring_ascii to correct this, but currently json is not a hard dependency which complicates things a little.

[Martin Pool (mbp)]

This patch improves it so that the tests at least pass. Things may still break on some input data, though, so it might be better to use a real parser as bug 871451 suggests.

[John A Meinel (jameinel)]

For the location of where it is being done, I think it would be fine to just use the json/simplejson module here. The only place that actively uses the code is in "remove_expensive_references", as that is the only place that writes out a new file.

The test suite passes here, so I'm not sure what the specific problem is, but I think just depending on json is reasonable at this point. (The compatibility hacks have long since served their purpose.)

[Martin Pool (mbp)]

I guess it's passing because you have simplejson on your path? I think it
will reliably fail if you don't. The reason is that in the regexp used
before this patch, the .* will consume the trailing quote, like in this
simplified example:

>>> re.match('"?(.*)"?', '"hello"').groups(1)
('hello"',)

I can do another patch that just cuts out the fallback and requires a
proper json parser. Do you want to require simplejson, or have a fallback
to Python2.6's standard json? <http://docs.python.org/2/library/json.html>

--
Martin

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-02-20 11:15, Martin Pool wrote:
> I guess it's passing because you have simplejson on your path? I
> think it will reliably fail if you don't. The reason is that in
> the regexp used before this patch, the .* will consume the trailing
> quote, like in this simplified example:
>
>>>> re.match('"?(.*)"?', '"hello"').groups(1)
> ('hello"',)
>
> I can do another patch that just cuts out the fallback and requires
> a proper json parser. Do you want to require simplejson, or have a
> fallback to Python2.6's standard json?
> <http://docs.python.org/2/library/json.html>
>

A fallback is fine. 'json' gets better with newer versions, I think
with 2.6 it was pure-python only, but we can live with that.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlEreYgACgkQJdeBCYSNAAMW4gCgwE6hyCbJpi5B+cZoLTr1HHdg
M8EAnAos/hF0+Vg3RiAJCGbD7/6EXXmK
=htff
-----END PGP SIGNATURE-----

[Martin Pool (mbp)]

John, do you have any opinion on whether we should prefer simplejson or
json?

I would tend to prefer using the standard library one. I don't know if
there are any substantial performance or robustness differences.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-02-26 6:25, Martin Pool wrote:
> John, do you have any opinion on whether we should prefer
> simplejson or json?
>
> I would tend to prefer using the standard library one. I don't
> know if there are any substantial performance or robustness
> differences.

In python 2.6 there are reasons to prefer simplejson (no C
accelerator), however I don't think it is worth worrying about rather
than just using whatever is available.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlEsSE8ACgkQJdeBCYSNAAMyeQCfUwh6wWTvWQvt7vhiWNyq3vK5
QdwAoK+h/0sELPftQS+YfUqcPJ2Dg3eX
=N1je
-----END PGP SIGNATURE-----

[Martin Pool (mbp)]

Hi John,

Here is an updated version of this patch that fixes the case where there is no json library. It's actually a bit obsolete since I've changed our build to make sure it can find one, but since this was mostly done I thought I might as well send it.

I'm not 100% sure the fix is covering what you intend for the test to do but I think it's correct.

By the way, people here are finding meliae really useful for Python memory problems - thanks!

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-05-14 12:16, Martin Pool wrote:
> Hi John,
>
> Here is an updated version of this patch that fixes the case where
> there is no json library. It's actually a bit obsolete since I've
> changed our build to make sure it can find one, but since this was
> mostly done I thought I might as well send it.
>
> I'm not 100% sure the fix is covering what you intend for the test
> to do but I think it's correct.
>
> By the way, people here are finding meliae really useful for Python
> memory problems - thanks!
>
> ** Patch added: "20130220-stringquote.diff"
> https://bugs.launchpad.net/meliae/+bug/871450/+attachment/3676157/+files/20130220-stringquote.diff
>
>
>
I'm glad you're finding it useful. I'm a bit surprised you're
submitting a patch rather than a branch, though. :)

Landed on trunk.

 status: fixcommitted

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlGYbaYACgkQJdeBCYSNAAMyewCgjLLiky14WLaEZZ7T/96em+7v
n+wAoJU4a1y2F7wp9hDr6AEHva3lXzBM
=Sb1N
-----END PGP SIGNATURE-----