Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON Parser Test Suite #232

Merged
merged 14 commits into from
Jan 8, 2025
Merged

JSON Parser Test Suite #232

merged 14 commits into from
Jan 8, 2025

Conversation

cpb8010
Copy link
Contributor

@cpb8010 cpb8010 commented Dec 26, 2024

Description

Adding test suite to json parsing library.

Additional context

While the client data JSON isn't expected to field the broad range of formats, it can be helpful to know what's generlaly missing.

The truffle tests were pretty minimal, but good to keep in sync since
this isn't a published package we're importing.
https://github.com/chrisdotn/jsmnSol/tree/master/test
Since the existing test suite didn't catch any of the reported problems
with the JSON parsing, adding another larger suite that's used to
compare popular JSON parsers should!

Obviously because this is so large it can be trimmed, but the hope is to
find overlap between the feedback and the test-cases here.
This action is broken again, hopefully the latest works!
Tests connect to the files
Skipping all the json format tests for now,
These are helpful if we want a fully compliant parser, but aren't
helpful in parsing the client data JSON.
It doesn't return an error, because it doesn't actually record key
values
@cpb8010 cpb8010 changed the title Json-tests JSON Parser Test Suite Jan 1, 2025
@cpb8010 cpb8010 self-assigned this Jan 1, 2025
it looks like this is fixed now
@cpb8010 cpb8010 requested review from itsacoyote and ly0va January 2, 2025 15:21
test/JsmnSolLib.ts Outdated Show resolved Hide resolved
test/JsmnSolLib.ts Outdated Show resolved Hide resolved
test/JsmnSolLib.ts Outdated Show resolved Hide resolved
test/JsmnSolLib.ts Show resolved Hide resolved
Copy link
Member

@ly0va ly0va left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to merge this with some parsing tests skipped and then decide if they're important enough to fix

@ly0va ly0va merged commit 8cef3bc into main Jan 8, 2025
3 checks passed
@ly0va ly0va deleted the json-tests branch January 8, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants