-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add ZerionBalancesSchema tests #1637
Conversation
@@ -19,41 +19,41 @@ export type ZerionBalance = z.infer<typeof ZerionBalanceSchema>; | |||
|
|||
export type ZerionBalances = z.infer<typeof ZerionBalancesSchema>; | |||
|
|||
const ZerionImplementationSchema = z.object({ | |||
export const ZerionImplementationSchema = z.object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The schemas in this file are exported to test them on the ad hoc tests.
const fiatBalance = value ? getNumberString(value) : null; | ||
const fiatConversion = getNumberString(zb.attributes.price); | ||
const fiatConversion = price ? getNumberString(price) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attributes.price
is not marked as required in the Zerion API docs, so I've changed this to reflect the possibility of the field being undefined
.
Pull Request Test Coverage Report for Build 9461883835Details
💛 - Coveralls |
it('should not allow an invalid type value', () => { | ||
const zerionBalance = zerionBalanceBuilder().build(); | ||
// @ts-expect-error - type is expected to be a 'positions' literal | ||
zerionBalance.type = 'invalid'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about adding an unknown
literal here if it throws?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in efadc24
chain_id: z.string(), | ||
address: z.string().nullable(), | ||
address: z.string().nullish().default(null), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use AddressSchema
here (and add a test for checksumming).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in efadc24
// @ts-expect-error - id is expected to be a string | ||
zerionBalance.id = faker.number.int(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially going to suggest we move this into the builder but this is a much more future-proof method of overwriting the values that we should consider doing elsewhere in schema tests - if the type changes to match that expected, TypeScript will complain. (If we casted it inside the with
of a builder, it wouldn't.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes perfect sense to me! 👍🏻
Pull Request Test Coverage Report for Build 9468416367Details
💛 - Coveralls |
Changes
ZerionBalances
entity schema unit tests.nullable()
bynullish().default(null)
for several fields in theZerionBalance
schema, following the Zerion API documentation.