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

Upgrading JSON IO tests to the new API, and removing the deprecated API #2104

Merged
merged 30 commits into from
Dec 31, 2024

Conversation

jurgenvinju
Copy link
Member

@jurgenvinju jurgenvinju commented Dec 18, 2024

  • roundtripping tests now use the new functions asJSON, parseJSON, readJSON and writeJSON
  • implemented roundtripping of datetime values by reusing features from the DateTime module
  • removed old toJSON and fromJSON functions (had been deprecated for 8 years) from lang::json::IO and removed their respective implementation classes in lang/json/io/internal
  • fixed roundtripping of rational numbers by encoding them as arrays of 2 integers
  • fixed roundtripping of node values by encoding their name as a _name field of the object.
  • fixed roundtripping with num
  • fixed roundtripping of sets, maps and tuples when the expected type is value by normalizing the expected values to the defaults that are chosen when no more specific type is requested
  • added syntax for empty tuple expressions and empty tuple patterns because empty tuples are generated by QuickCheck
  • nodes with field names that can't become (or could never have been) typescript or javascript object names are read back as maps
  • normalization for all ambiguous JSON objects is complete: maps, objects, sets, lists. This makes it easy to test the asJSON/parseJSON pair for abstract expected types like num, value and node
  • added documentation on the bi-directionality of asJSON, parseJSON and readJSON, writeJSON
  • allow serialization of bigger or more accurate numbers than double and long
  • create general canonicalization function for testing roundtrip equality of abstract types which also accounts for accidental new key collisions in maps.
  • all tests succeed

Copy link

codecov bot commented Dec 24, 2024

Codecov Report

Attention: Patch coverage is 67.79661% with 19 lines in your changes missing coverage. Please review.

Project coverage is 49%. Comparing base (cfc601c) to head (b188a8f).
Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
...interpreter/utils/IUPTRAstToSymbolConstructor.java 0% 7 Missing ⚠️
...pl/library/lang/json/internal/JsonValueWriter.java 75% 6 Missing ⚠️
...pl/library/lang/json/internal/JsonValueReader.java 73% 3 Missing and 2 partials ⚠️
src/org/rascalmpl/ast/Expression.java 83% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##              main   #2104    +/-   ##
========================================
  Coverage       49%     49%            
+ Complexity    6336    6234   -102     
========================================
  Files          667     663     -4     
  Lines        59836   59181   -655     
  Branches      8692    8615    -77     
========================================
- Hits         29718   29475   -243     
+ Misses       27868   27475   -393     
+ Partials      2250    2231    -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

LGTM, I have a few small concerns, but I'm very happy to see a whole bunch of old code get cleaned up 👍🏼

@jurgenvinju
Copy link
Member Author

I want to increase the test coverage a bit before merging this

@jurgenvinju
Copy link
Member Author

Test coverage can be increased a lot by testing the different nul representations. I'm leaving that for another PR since this one is already quite big.

@jurgenvinju jurgenvinju merged commit 6c58baa into main Dec 31, 2024
7 checks passed
@jurgenvinju jurgenvinju deleted the fortify-json-tests branch December 31, 2024 12:49
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.

2 participants