Skip to content

test(ui): consolidate SDK test helpers - #6931

Merged
otavio merged 2 commits into
masterfrom
test/ui/cleanup
Aug 20, 2026
Merged

test(ui): consolidate SDK test helpers#6931
otavio merged 2 commits into
masterfrom
test/ui/cleanup

Conversation

@luizhf42

Copy link
Copy Markdown
Member

What

Consolidated duplicated SDK test factories into canonical locations and aligned SdkResponse with the actual hey-api client shape.

Why

Prerequisite cleanup for shellhub-io/team#216 (Task 5 — migrating page/component tests to mock @/client). Reduces the number of ad-hoc error factories tests need to know about before the migration touches every test file.

Changes

  • factories.ts: renamed from userAuth.ts to host all API type factory functions (mockUserAuth today, mockTag/mockNamespace next); updated 3 consumers (Login.test.tsx, authStore.test.ts, mfaResetStore.test.ts)
  • sdkError.ts deleted: duplicated makeSdkError from sdk.ts without the SdkHttpError type or header support; 8 page tests migrated to makeSdkError
  • SdkResponse: added error: undefined to match the hey-api RequestResult discriminated union ({ data, error, request, response })
  • makeSdkError: return type widened to Error & SdkHttpError so it satisfies both SdkHttpError and React Query's Error | null error field on hook mocks (temporary — the Error side becomes irrelevant once page tests stop mocking hooks)

Centralizes API type factories in a single file so `mockTag`,
`mockNamespace`, and future factories land alongside `mockUserAuth`.
…Response`

`sdkError.ts` duplicated `makeSdkError` from `sdk.ts` without the
`SdkHttpError` type or header support. Delete it and point all
consumers at `makeSdkError`.

`SdkResponse` now includes `error: undefined` to match the actual
hey-api client shape (`{ data, error, request, response }`).
@luizhf42
luizhf42 requested a review from a team as a code owner August 20, 2026 14:42
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

Claude finished @luizhf42's task in 58s —— View job


Code Review Complete

Reviewed 15 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

A few notes on why the change is sound:

  • makeSdkError widening to Error & SdkHttpError and the removal of sdkError.ts are safe: every migrated call site only passed a status (sdkError(500) / sdkError(403)), and apiErrorMessage keys solely off status, which both factories provide.
  • Adding error: undefined to SdkResponse / mockSdkResponse correctly aligns the success arm of the hey-api RequestResult discriminated union, and the new sdk.test.ts assertion covers it.
  • No remaining references to @/tests/sdkError or @/tests/userAuth — the leftover sdkError matches elsewhere are unrelated local variables.

If you push additional changes and want a new review, tag @shellhub-io/admin and a team member can trigger it.
· test/ui/cleanup

@luizhf42 luizhf42 self-assigned this Aug 20, 2026
@otavio
otavio merged commit 6a91fce into master Aug 20, 2026
70 checks passed
@otavio
otavio deleted the test/ui/cleanup branch August 20, 2026 15:43
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.

2 participants