Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

WIP: Fix CPython tests #54

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

WIP: Fix CPython tests #54

wants to merge 10 commits into from

Conversation

mre
Copy link
Owner

@mre mre commented Nov 26, 2018

See #8

@mre
Copy link
Owner Author

mre commented Nov 26, 2018

@konstin thanks to the fix for PyO3/pyo3#177 I was able to add our own JSONDecodeError to hyperjson (yay!).
This fixes a few unit tests and that makes me very happy.

One issue that keeps popping up is, that the msg field is missing from our JSONDecodeError:

    def test_unexpected_data(self):
        test_cases = [
            ('[,', 'Expecting value', 1),
            ('{"spam":[}', 'Expecting value', 9),
            ('[42:', "Expecting ',' delimiter", 3),
            ('[42 "spam"', "Expecting ',' delimiter", 4),
            ('[42,]', 'Expecting value', 4),
            ('{"spam":[42}', "Expecting ',' delimiter", 11),
            ('["]', 'Unterminated string starting at', 1),
            ('["spam":', "Expecting ',' delimiter", 7),
            ('["spam",]', 'Expecting value', 8),
            ('{:', 'Expecting property name enclosed in double quotes', 1),
            ('{,', 'Expecting property name enclosed in double quotes', 1),
            ('{42', 'Expecting property name enclosed in double quotes', 1),
            ('[{]', 'Expecting property name enclosed in double quotes', 2),
            ('{"spam",', "Expecting ':' delimiter", 7),
            ('{"spam"}', "Expecting ':' delimiter", 7),
            ('[{"spam"]', "Expecting ':' delimiter", 8),
            ('{"spam":}', 'Expecting value', 8),
            ('[{"spam":]', 'Expecting value', 9),
            ('{"spam":42 "ham"', "Expecting ',' delimiter", 11),
            ('[{"spam":42]', "Expecting ',' delimiter", 11),
            ('{"spam":42,}', 'Expecting property name enclosed in double quotes', 11),
        ]
        for data, msg, idx in test_cases:
            with self.assertRaises(self.JSONDecodeError) as cm:
                self.loads(data)
            err = cm.exception
>           self.assertEqual(err.msg, msg)
E           AttributeError: 'JSONDecodeError' object has no attribute 'msg'

I think, fixing that would make quite a few more tests pass.
The question is, what's the most idiomatic way to add the msg attribute here?
I could go and copy the output of the py_exception! macro to the code, but I wonder if there's a better way.

One thing that would be really fancy is supporting custom derives on structs to make them Python exceptions. Something like

#[derive(PyException)]
struct JSONDecodeError {
  msg: String,
}

Have you considered this? I'm a bit reluctant to open an issue on pyo3, because... well... it's not really an issue. 😉

@konstin
Copy link
Collaborator

konstin commented Nov 27, 2018

The easiest fix is to just add a msg field to the python object by name. Otherwise with PyO3/pyo3#291 everything is ready to implement a custom (Which should make a Good First Issue)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants