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

All unit tests must pass in strict mode #891

Open
stleary opened this issue Apr 28, 2024 · 4 comments
Open

All unit tests must pass in strict mode #891

stleary opened this issue Apr 28, 2024 · 4 comments

Comments

@stleary
Copy link
Owner

stleary commented Apr 28, 2024

The unit tests should yield the same result for default and strict mode parsing.
Tests may fail for the following reasons:

  • The test data inadvertently does not include double quotes around some string values. These should be fixed.
  • The test is specifically intended to test the parsing of unquoted chars. These should be omitted from the strict mode test run.
  • The test has specific conditions (e.g. working with unquoted strings for CDL or XML testing) that are impacted by the strict mode parser changes.

The unit tests will be updated as a separate issue from #887 and #888. After merging the unit test fixes, the review of strict mode may continue.

@rikkarth
Copy link
Contributor

All passing except for this one case that we're still debating.
#888 (comment)

@stleary
Copy link
Owner Author

stleary commented May 17, 2024

@rikkarth No worries, I will go ahead and accept this PR and merge it in 3 days. Will also hold off on merging #894 so that you don't have yet another commit to merge to.
However, your code still has this line in JSONParserConfiguration:
this.strictMode = true;
image

This must be deleted or at least commented out. You can do it yourself if you have time, or I can edit the file in place.
I also think that smallCharMemory and arrayLevel can still be optimized away, but that is a task for another day.
Thanks for all of the work you put into #888!

@rikkarth
Copy link
Contributor

rikkarth commented May 17, 2024

I will clean it up for you. Thank you @stleary

If you have some tests that need optimization, review or implementation let me know I can help in doing some.

@rikkarth
Copy link
Contributor

Merged.

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

No branches or pull requests

2 participants