-
Notifications
You must be signed in to change notification settings - Fork 0
Add unit tests for untested utility functions #11
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { toBigintOrThrow } from '../src/utils/bigint'; | ||
| import { SdkError } from '../src/errors'; | ||
|
|
||
| const ctx = { code: 'CONFIG' as const, name: 'amount', detail: {} }; | ||
|
|
||
| describe('toBigintOrThrow', () => { | ||
| it('passes through bigint values', () => { | ||
| expect(toBigintOrThrow(42n, ctx)).toBe(42n); | ||
| expect(toBigintOrThrow(0n, ctx)).toBe(0n); | ||
| }); | ||
|
|
||
| it('converts numeric strings', () => { | ||
| expect(toBigintOrThrow('123', ctx)).toBe(123n); | ||
| expect(toBigintOrThrow('0', ctx)).toBe(0n); | ||
| }); | ||
|
|
||
| it('converts finite numbers', () => { | ||
| expect(toBigintOrThrow(100, ctx)).toBe(100n); | ||
| expect(toBigintOrThrow(0, ctx)).toBe(0n); | ||
| }); | ||
|
|
||
| it('throws SdkError for invalid input', () => { | ||
| expect(() => toBigintOrThrow('not-a-number', ctx)).toThrow(SdkError); | ||
| expect(() => toBigintOrThrow(null, ctx)).toThrow(SdkError); | ||
| expect(() => toBigintOrThrow(undefined, ctx)).toThrow(SdkError); | ||
| expect(() => toBigintOrThrow({}, ctx)).toThrow(SdkError); | ||
| }); | ||
|
|
||
| it('includes field name in error message', () => { | ||
| try { | ||
| toBigintOrThrow('bad', ctx); | ||
| } catch (err) { | ||
| expect(err).toBeInstanceOf(SdkError); | ||
| expect((err as SdkError).message).toContain('amount'); | ||
| expect((err as SdkError).code).toBe('CONFIG'); | ||
| } | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { isHexStrict } from '../src/utils/hex'; | ||
|
|
||
| describe('isHexStrict', () => { | ||
| it('accepts valid hex strings', () => { | ||
| expect(isHexStrict('0xab')).toBe(true); | ||
| expect(isHexStrict('0xABCDEF0123456789')).toBe(true); | ||
| expect(isHexStrict('0x00')).toBe(true); | ||
| }); | ||
|
|
||
| it('rejects non-string input', () => { | ||
| expect(isHexStrict(42)).toBe(false); | ||
| expect(isHexStrict(null)).toBe(false); | ||
| expect(isHexStrict(undefined)).toBe(false); | ||
| expect(isHexStrict({})).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects strings without 0x prefix', () => { | ||
| expect(isHexStrict('abcd')).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects empty payload (bare 0x)', () => { | ||
| expect(isHexStrict('0x')).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects odd-length payload', () => { | ||
| expect(isHexStrict('0xabc')).toBe(false); | ||
| expect(isHexStrict('0x1')).toBe(false); | ||
| }); | ||
|
|
||
| it('rejects non-hex characters', () => { | ||
| expect(isHexStrict('0xgh')).toBe(false); | ||
| expect(isHexStrict('0x00zz')).toBe(false); | ||
| }); | ||
|
|
||
| it('enforces minBytes option', () => { | ||
| expect(isHexStrict('0xab', { minBytes: 1 })).toBe(true); | ||
| expect(isHexStrict('0xab', { minBytes: 2 })).toBe(false); | ||
| expect(isHexStrict('0xaabbccdd', { minBytes: 4 })).toBe(true); | ||
| expect(isHexStrict('0xaabb', { minBytes: 4 })).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||||||||||
| import { describe, expect, it } from 'vitest'; | ||||||||||||||||||||||||
| import { bigintReplacer, serializeBigInt, stableStringify } from '../src/utils/json'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('bigintReplacer', () => { | ||||||||||||||||||||||||
| it('converts bigint to string', () => { | ||||||||||||||||||||||||
| expect(bigintReplacer('x', 42n)).toBe('42'); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('passes non-bigint values through', () => { | ||||||||||||||||||||||||
| expect(bigintReplacer('x', 'hello')).toBe('hello'); | ||||||||||||||||||||||||
| expect(bigintReplacer('x', 99)).toBe(99); | ||||||||||||||||||||||||
| expect(bigintReplacer('x', null)).toBe(null); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('serializeBigInt', () => { | ||||||||||||||||||||||||
| it('serializes objects with bigint fields', () => { | ||||||||||||||||||||||||
| const result = serializeBigInt({ a: 1n, b: 'text' }); | ||||||||||||||||||||||||
| expect(JSON.parse(result)).toEqual({ a: '1', b: 'text' }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| describe('stableStringify', () => { | ||||||||||||||||||||||||
| it('sorts object keys for deterministic output', () => { | ||||||||||||||||||||||||
| const a = stableStringify({ z: 1, a: 2 }); | ||||||||||||||||||||||||
| const b = stableStringify({ a: 2, z: 1 }); | ||||||||||||||||||||||||
| expect(a).toBe(b); | ||||||||||||||||||||||||
| expect(JSON.parse(a)).toEqual({ a: 2, z: 1 }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('handles nested objects with sorted keys', () => { | ||||||||||||||||||||||||
| const result = stableStringify({ b: { d: 1, c: 2 }, a: 3 }); | ||||||||||||||||||||||||
| const keys = Object.keys(JSON.parse(result)); | ||||||||||||||||||||||||
| expect(keys).toEqual(['a', 'b']); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
Comment on lines
+31
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test case only verifies that the top-level keys are sorted. It doesn't check if the keys of nested objects are also sorted, which is part of the function's intended behavior. To properly test this, you should compare the stringified output of objects with differently ordered nested keys or assert against the exact expected string output.
Suggested change
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('handles arrays without reordering', () => { | ||||||||||||||||||||||||
| const result = stableStringify([3, 1, 2]); | ||||||||||||||||||||||||
| expect(JSON.parse(result)).toEqual([3, 1, 2]); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('converts bigint values', () => { | ||||||||||||||||||||||||
| const result = stableStringify({ amount: 100n }); | ||||||||||||||||||||||||
| expect(JSON.parse(result)).toEqual({ amount: '100' }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('strips undefined values', () => { | ||||||||||||||||||||||||
| const result = stableStringify({ a: 1, b: undefined }); | ||||||||||||||||||||||||
| expect(JSON.parse(result)).toEqual({ a: 1 }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| it('handles null and primitives', () => { | ||||||||||||||||||||||||
| expect(stableStringify(null)).toBe('null'); | ||||||||||||||||||||||||
| expect(stableStringify(42)).toBe('42'); | ||||||||||||||||||||||||
| expect(stableStringify('hi')).toBe('"hi"'); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { signalTimeout, signalAny } from '../src/utils/signal'; | ||
|
|
||
| describe('signalTimeout', () => { | ||
| it('returns an AbortSignal', () => { | ||
| const signal = signalTimeout(5_000); | ||
| expect(signal).toBeInstanceOf(AbortSignal); | ||
| expect(signal.aborted).toBe(false); | ||
| }); | ||
|
|
||
| it('aborts after the specified timeout', async () => { | ||
| const signal = signalTimeout(50); | ||
| expect(signal.aborted).toBe(false); | ||
| await new Promise((r) => setTimeout(r, 100)); | ||
| expect(signal.aborted).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('signalAny', () => { | ||
| it('returns undefined for empty input', () => { | ||
| expect(signalAny([])).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('returns undefined when all entries are undefined', () => { | ||
| expect(signalAny([undefined, undefined])).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('aborts when any input signal aborts', () => { | ||
| const c1 = new AbortController(); | ||
| const c2 = new AbortController(); | ||
| const combined = signalAny([c1.signal, c2.signal]); | ||
| expect(combined).toBeDefined(); | ||
| expect(combined!.aborted).toBe(false); | ||
|
|
||
| c1.abort(new Error('first')); | ||
| expect(combined!.aborted).toBe(true); | ||
| }); | ||
|
Comment on lines
+28
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While this test correctly verifies that the combined signal aborts, it doesn't cover a potential memory leak in the A more robust implementation would clean up all listeners once the combined signal is aborted. While this is difficult to test without mocking, it's a significant issue in the underlying implementation that these tests are covering. |
||
|
|
||
| it('is already aborted if an input signal is pre-aborted', () => { | ||
| const c = new AbortController(); | ||
| c.abort(new Error('already')); | ||
| const combined = signalAny([c.signal]); | ||
| expect(combined!.aborted).toBe(true); | ||
| }); | ||
| }); | ||
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.
Using a
try...catchblock to assert on thrown errors can be brittle. IftoBigintOrThrow('bad', ctx)doesn't throw, this test will pass silently, which is incorrect. It's better to usevitest'sexpect().toThrow()with an error object matcher for a more robust and concise test.