Skip to content

Conversation

@andrewkmin
Copy link
Contributor

@andrewkmin andrewkmin commented Aug 7, 2024

$title

also formats the test file. perhaps format everything (e.g. via prettier) in a separate, standalone PR

}).toThrow("number cannot fit in a hex string of 2 characters");
});

it("contains bigIntToBase64Url", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the rest of the above is formatting; this is the net-new test suite collection that needs review :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's a separate PR to deal with just prettifying: #50

expect(() => { TKHQ.bigIntToHex(BigInt(256), 2) }).toThrow("number cannot fit in a hex string of 2 characters");
})
expect(() => {
TKHQ.bigIntToHex(BigInt(256), 2);
Copy link
Contributor

@zkharit zkharit Aug 8, 2024

Choose a reason for hiding this comment

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

want to add a similar expect(TKHQ.bigIntToBase64Url(BigInt(255), 2)).toThrow("number cannot fit in a hex string of 2 characters"); under the other bigIntToBase64Url tests for completeness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call!

zkharit
zkharit previously approved these changes Aug 8, 2024
omkarshanbhag
omkarshanbhag previously approved these changes Aug 8, 2024
@andrewkmin andrewkmin force-pushed the andrew/test-int-base64url branch from ebfd9da to 52baccc Compare August 9, 2024 20:40
@andrewkmin andrewkmin force-pushed the andrew/test-int-base64url branch from 52baccc to 2316ccd Compare August 9, 2024 20:43
@andrewkmin andrewkmin dismissed stale reviews from omkarshanbhag and zkharit via 2316ccd October 1, 2025 19:38
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.

4 participants