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

update tests with additional tests and snapshots using v4 #274

Merged
merged 3 commits into from
Nov 9, 2022

Conversation

adonesky1
Copy link
Contributor

@adonesky1 adonesky1 commented Oct 10, 2022

Adds tests for encoded data types of hexadecimal strings that convert to numbers equal to or greater than Javascript's MAX_SAFE_INTEGER. The following additional test cases are added to each encoded field "type":

  1. 0x1fffffffffffff - The hex string equal to Number.MAX_SAFE_INTEGER, which contains an even number (16) of characters
  2. 0x1fffffffffffff1 a hex string that contains an odd number of characters (17) and is greater than Number.MAX_SAFE_INTEGER.
  3. 0x101 a hex string that contains an odd number of characters and is less than Number.MAX_SAFE_INTEGER

We test both even and odd length hex values because Node's Buffer.from() method does not buffer hex numbers correctly
so we must conditionally prepend hexstrings with a zero before buffering them depending on whether the string contains an
even or odd number of characters.

The added snapshots were taken by adding the additional test values to 219511e, the commit of the last release before v5 caused regressions to encoding of buffered hex strings.

You can manually test this by adding the new tests to a previous release (4.0.0 or 4.0.1), update the snapshots (yarn jest -u). Copy the updated snapshot file to this branch and run the tests to ensure that v5 matches the expected output for encoding these values.

@adonesky1 adonesky1 marked this pull request as ready for review November 8, 2022 22:29
@adonesky1 adonesky1 requested a review from a team as a code owner November 8, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants