Conversation
|
I know it isn't the prettiest code but for now I'm tired and don't have energy for it |
Rei-x
left a comment
There was a problem hiding this comment.
spoczko testy dodi, raczej nie mam jakichś dużych uwag, są w większości bardzo poprawne i testują co mają testować - może brakuje mi jakichś testów logiki biznesowej albo happy patha, w większości przetestowałeś ekrany błędów
| vi.mock("@/hooks/use-share", () => ({ | ||
| useShare: () => ({ | ||
| isDialogOpen: false, | ||
| openDialog: vi.fn(), | ||
| closeDialog: vi.fn(), | ||
| setIsDialogOpen: vi.fn(), | ||
| }), | ||
| })); |
There was a problem hiding this comment.
nie trzeba chyba tego mockować? mogłeś to jakoś ograć kodem, staraj się mockować jak najmniej
| beforeEach(() => { | ||
| localStorage.setItem( | ||
| "plansIds-v2", | ||
| JSON.stringify([{ id: "a5e9b042-9151-4719-9f1d-8ff6fb216900" }]), |
There was a problem hiding this comment.
wydzieliłbym to id do jakiejś zmiennej, żeby było wiadomo co go używa
| localStorage.clear(); | ||
| }); | ||
|
|
||
| it("Should display error for faculties without backend", async () => { |
There was a problem hiding this comment.
raczej bym ustawił i tak jakiś handler w msw, ale taki co zwróci 500, zamiast żadnego - jesteś wtedy explicit co testujesz
There was a problem hiding this comment.
teraz zauważyłem, że są gdzieś w setupie - nie jestem tego fanem, bo (tak jak mi przed chwilą) ciężko zrozumieć co się właściwie dzieje - najlepiej ustawiaj request handlery w teście
| const btns = screen.getAllByRole("button"); | ||
| await user.click(btns[0]); |
There was a problem hiding this comment.
oj oj, to jest bardzo wrażliwy locator, ktoś doda drugi przycisk i nagle ten test pada
|
|
||
| globalThis.ResizeObserver = ResizeObserver; | ||
|
|
||
| const takeRecordsMock = () => []; |
There was a problem hiding this comment.
może jestem ślepy, ale gdzie to jest używane?
There was a problem hiding this comment.
Kurde nie pamiętam, ale coś mi sie wywalało bez obserwera
There was a problem hiding this comment.
w sensie nie observer, tylko takeRecordsMock
| import { HttpResponse, http } from "msw"; | ||
|
|
||
| export const handlers = [ | ||
| http.get("http://localhost:3333/api/departments", () => { |
There was a problem hiding this comment.
raczej bym używał zmiennej z env'a zamiast hardkodować localhosta
|
|
||
| afterEach(() => { | ||
| localStorage.clear(); | ||
| vi.clearAllMocks(); |
There was a problem hiding this comment.
możesz to wrzucić do configu vitesta - cleanMocks
There was a problem hiding this comment.
ja akurat jestem fanem kolokacji testów czyli ten test bym wrzucił w @/app/plans/edit/[id]/page.client.test.tsx
There was a problem hiding this comment.
Hm okej w sumie spoko pomysł, ale nie lepiej jeszcze bardziej do: @/app/plans/edit/[id]/tests/page.client.test.tsx?
There was a problem hiding this comment.
jak wolisz, ja zwykle nie robię dodatkowych folderów - i też nie spotkałem się jeszcze z takim podejściem jak ty opisałeś
There was a problem hiding this comment.
zazwyczaj albo ludzie robią pliki koło siebie albo moję kompletnie osobny folder tests koło src
There was a problem hiding this comment.
Okej, po prostu to wciskanie testu obok stron mnie trochę gryzie, a co np. jeśliby zrobić feature based architekturę(wiem, że w planerze tego nie mamy) i mieć np. features/create-plan/tests to chyba by fajnie wyglądało
There was a problem hiding this comment.
noo jak lubisz, raczej ludzie robią pliki koło sieibe
Adding tests setup
Important
Introduces Vitest, React Testing Library, and MSW for testing with new test files, mock handlers, and configuration updates.
vitest,react-testing-library, andmswfor testing.vitest.config.mtsfor Vitest configuration withjsdomenvironment and setup files.plans-edit.test.tsxto test error handling inCreateNewPlanPage.plans.test.tsxto test error handling inPlanItem.handlers.tsandserver.tsfor MSW mock server setup.package.jsonscripts to includevitestcommands for testing.app-sidebar.tsxfor registration fetch failures.setup.tsfor test environment setup with global mocks and server lifecycle management.This description was created by
for 463a6dd. You can customize this summary. It will automatically update as commits are pushed.