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

Added more serializer/deserializer tests. #559

Closed
wants to merge 1 commit into from

Conversation

dblock
Copy link
Member

@dblock dblock commented Jul 13, 2023

Description

Coming from #558. Are there cases that would still fail?

Check List

  • New functionality includes testing.
    • All tests pass
  • Linter check was successfull - yarn run lint doesn't show any errors
  • Commits are signed per the DCO using --signoff
  • Changelog was updated.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dblock dblock changed the title Added more deserializer tests. Added more serializer/deserializer tests. Jul 13, 2023
`"a": "෴1111111111111111",` +
`"b": "߷1111111111111111",` +
`"c": "֍1111111111111111",` +
`"d": "෴֍߷1111111111111111"` +
`}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you want to add the expected outputs like t.equal(obj.hardcoded, 102931203123987); on line 71?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I will be adding similar ones in my other PR. I would recommend we hold off on these.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@dblock dblock force-pushed the test-deserializer branch from ba94412 to c1d6ab4 Compare July 14, 2023 17:10
@dblock
Copy link
Member Author

dblock commented Jul 14, 2023

@AMoo-Miki feel free to close this one if you have better

@dblock
Copy link
Member Author

dblock commented Jul 14, 2023

#557 is better.

@dblock dblock closed this Jul 14, 2023
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