From 5977b9da8726cbc70a14f8a924dedf82aac03f41 Mon Sep 17 00:00:00 2001 From: Joshua Horowitz Date: Sun, 10 Mar 2024 00:18:28 -0800 Subject: [PATCH] tests: finish moving away from renderHook --- .../test/useDocument.test.tsx | 60 +++++++-------- .../test/useDocuments.test.tsx | 30 ++++---- .../test/useHandle.test.tsx | 75 +++++++------------ .../test/useRepo.test.tsx | 21 ++++-- 4 files changed, 89 insertions(+), 97 deletions(-) diff --git a/packages/automerge-repo-react-hooks/test/useDocument.test.tsx b/packages/automerge-repo-react-hooks/test/useDocument.test.tsx index fe5eb5c1d..d5bd7ae27 100644 --- a/packages/automerge-repo-react-hooks/test/useDocument.test.tsx +++ b/packages/automerge-repo-react-hooks/test/useDocument.test.tsx @@ -48,80 +48,80 @@ describe("useDocument", () => { } } - const Component = ({ url, onRender }: { + const Component = ({ url, onDoc }: { url: AutomergeUrl, - onRender: (doc: ExampleDoc) => void, + onDoc: (doc: ExampleDoc) => void, }) => { const [doc] = useDocument(url) - onRender(doc) + onDoc(doc) return null } it("should load a document", async () => { const { handleA, wrapper } = setup() - const onRender = vi.fn() + const onDoc = vi.fn() - render(, {wrapper}) - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "A" })) + render(, {wrapper}) + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "A" })) }) it("should update if the url changes", async () => { const { handleA, handleB, wrapper } = setup() - const onRender = vi.fn() + const onDoc = vi.fn() - const { rerender } = render(, {wrapper}) - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith(undefined)) + const { rerender } = render(, {wrapper}) + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith(undefined)) // set url to doc A - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "A" })) + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "A" })) // set url to doc B - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "B" })) + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "B" })) // set url to undefined - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith(undefined)) + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith(undefined)) }) it("sets the doc to undefined while the initial load is happening", async () => { const { handleA, handleSlow, wrapper } = setup() - const onRender = vi.fn() + const onDoc = vi.fn() - const { rerender } = render(, {wrapper}) - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith(undefined)) + const { rerender } = render(, {wrapper}) + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith(undefined)) // start by setting url to doc A - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "A" })) + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "A" })) // Now we set the URL to a handle that's slow to load. // The doc should be undefined while the load is happening. - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith(undefined)) - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "slow" })) + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith(undefined)) + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "slow" })) }) it("avoids showing stale data", async () => { const { handleA, handleSlow, wrapper } = setup() - const onRender = vi.fn() + const onDoc = vi.fn() - const { rerender } = render(, {wrapper}) - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith(undefined)) + const { rerender } = render(, {wrapper}) + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith(undefined)) // Set the URL to a slow doc and then a fast doc. // We should see the fast doc forever, even after // the slow doc has had time to finish loading. - rerender() - rerender() - await waitFor(() => expect(onRender).toHaveBeenLastCalledWith({ foo: "A" })) + rerender() + rerender() + await waitFor(() => expect(onDoc).toHaveBeenLastCalledWith({ foo: "A" })) // wait for the slow doc to finish loading... await pause(SLOW_DOC_LOAD_TIME_MS * 2) // we didn't update the doc to the slow doc, so it should still be A - expect(onRender).not.toHaveBeenCalledWith({ foo: "slow" }) + expect(onDoc).not.toHaveBeenCalledWith({ foo: "slow" }) }) }) diff --git a/packages/automerge-repo-react-hooks/test/useDocuments.test.tsx b/packages/automerge-repo-react-hooks/test/useDocuments.test.tsx index 9516631fb..36e7bab4e 100644 --- a/packages/automerge-repo-react-hooks/test/useDocuments.test.tsx +++ b/packages/automerge-repo-react-hooks/test/useDocuments.test.tsx @@ -26,31 +26,31 @@ describe("useDocuments", () => { return { repo, wrapper, documentIds } } - const Component = ({ ids, onRender }: { + const Component = ({ ids, onDocs }: { ids: DocumentId[], - onRender: (documents: Record) => void, + onDocs: (documents: Record) => void, }) => { const documents = useDocuments(ids) - onRender(documents) + onDocs(documents) return null } it("returns a collection of documents, given a list of ids", async () => { const { documentIds, wrapper } = setup() - const onRender = vi.fn() + const onDocs = vi.fn() - render(, { wrapper }) - await waitFor(() => expect(onRender).toHaveBeenCalledWith( + render(, { wrapper }) + await waitFor(() => expect(onDocs).toHaveBeenCalledWith( Object.fromEntries(documentIds.map((id, i) => [id, { foo: i }])) )) }) it("updates documents when they change", async () => { const { repo, documentIds, wrapper } = setup() - const onRender = vi.fn() + const onDocs = vi.fn() - render(, { wrapper }) - await waitFor(() => expect(onRender).toHaveBeenCalledWith( + render(, { wrapper }) + await waitFor(() => expect(onDocs).toHaveBeenCalledWith( Object.fromEntries(documentIds.map((id, i) => [id, { foo: i }])) )) @@ -61,26 +61,26 @@ describe("useDocuments", () => { handle.change(s => (s.foo *= 10)) }) }) - await waitFor(() => expect(onRender).toHaveBeenCalledWith( + await waitFor(() => expect(onDocs).toHaveBeenCalledWith( Object.fromEntries(documentIds.map((id, i) => [id, { foo: i * 10 }])) )) }) it(`removes documents when they're removed from the list of ids`, async () => { const { documentIds, wrapper } = setup() - const onRender = vi.fn() + const onDocs = vi.fn() - render(, { wrapper }) - await waitFor(() => expect(onRender).toHaveBeenCalledWith( + const { rerender } = render(, { wrapper }) + await waitFor(() => expect(onDocs).toHaveBeenCalledWith( Object.fromEntries(documentIds.map((id, i) => [id, { foo: i }])) )) // remove the first document - render(, { wrapper }) + rerender() // 👆 Note that this only works because documentIds.slice(1) is a different // object from documentIds. If we modified documentIds directly, the hook // wouldn't re-run. - await waitFor(() => expect(onRender).toHaveBeenCalledWith( + await waitFor(() => expect(onDocs).toHaveBeenCalledWith( Object.fromEntries(documentIds.map((id, i) => [id, { foo: i }]).slice(1)) )) }) diff --git a/packages/automerge-repo-react-hooks/test/useHandle.test.tsx b/packages/automerge-repo-react-hooks/test/useHandle.test.tsx index 965d0cdb3..910c423c3 100644 --- a/packages/automerge-repo-react-hooks/test/useHandle.test.tsx +++ b/packages/automerge-repo-react-hooks/test/useHandle.test.tsx @@ -1,11 +1,10 @@ -import { PeerId, Repo, AutomergeUrl } from "@automerge/automerge-repo" +import { AutomergeUrl, DocHandle, PeerId, Repo } from "@automerge/automerge-repo" import { DummyStorageAdapter } from "@automerge/automerge-repo/test/helpers/DummyStorageAdapter" -import { describe, expect, it } from "vitest" -import { RepoContext } from "../src/useRepo" +import { render, waitFor } from "@testing-library/react" +import React from "react" +import { describe, expect, it, vi } from "vitest" import { useHandle } from "../src/useHandle" -import { act, renderHook, waitFor } from "@testing-library/react" -import React, { useState } from "react" -import assert from "assert" +import { RepoContext } from "../src/useRepo" interface ExampleDoc { foo: string @@ -39,64 +38,48 @@ describe("useHandle", () => { } } + const Component = ({ url, onHandle }: { + url: AutomergeUrl, + onHandle: (handle: DocHandle | undefined) => void, + }) => { + const handle = useHandle(url) + onHandle(handle) + return null + } + it("loads a handle", async () => { const { handleA, wrapper } = setup() + const onHandle = vi.fn() - const { result } = await act(() => - renderHook( - () => { - const handle = useHandle(handleA.url) - return { handle } - }, - { wrapper } - ) - ) - - assert.deepStrictEqual(result.current.handle, handleA) + render(, {wrapper}) + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(handleA)) }) it("returns undefined when no url given", async () => { const { wrapper } = setup() + const onHandle = vi.fn() - const { result } = await act(() => - renderHook( - () => { - const handle = useHandle() - return { handle } - }, - { wrapper } - ) - ) - - await waitFor(() => expect(result.current.handle).toBeUndefined()) + render(, {wrapper}) + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(undefined)) }) it("updates the handle when the url changes", async () => { const { wrapper, handleA, handleB } = setup() + const onHandle = vi.fn() - const { result } = await act(() => - renderHook( - () => { - const [url, setUrl] = useState() - const handle = useHandle(url) - return { setUrl, handle } - }, - { wrapper } - ) - ) - - await waitFor(() => expect(result.current).not.toBeNull()) + const { rerender } = render(, {wrapper}) + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(undefined)) // set url to doc A - act(() => result.current.setUrl(handleA.url)) - await waitFor(() => expect(result.current.handle).toMatchObject(handleA)) + rerender() + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(handleA)) // set url to doc B - act(() => result.current.setUrl(handleB.url)) - await waitFor(() => expect(result.current.handle).toMatchObject(handleB)) + rerender() + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(handleB)) // set url to undefined - act(() => result.current.setUrl(undefined)) - await waitFor(() => expect(result.current.handle).toBeUndefined()) + rerender() + await waitFor(() => expect(onHandle).toHaveBeenLastCalledWith(undefined)) }) }) diff --git a/packages/automerge-repo-react-hooks/test/useRepo.test.tsx b/packages/automerge-repo-react-hooks/test/useRepo.test.tsx index 967736fbf..2269731bb 100644 --- a/packages/automerge-repo-react-hooks/test/useRepo.test.tsx +++ b/packages/automerge-repo-react-hooks/test/useRepo.test.tsx @@ -1,16 +1,24 @@ -import { renderHook } from "@testing-library/react" -import { describe, expect, test, vi } from "vitest" -import { RepoContext, useRepo } from "../src/useRepo.js" import { Repo } from "@automerge/automerge-repo" +import { render } from "@testing-library/react" import React from "react" +import { describe, expect, test, vi } from "vitest" +import { RepoContext, useRepo } from "../src/useRepo.js" describe("useRepo", () => { + const Component = ({ onRepo }: { + onRepo: (repo: Repo) => void, + }) => { + const repo = useRepo() + onRepo(repo) + return null + } + test("should error when context unavailable", () => { const repo = new Repo({ network: [] }) // Prevent console spam by swallowing console.error "uncaught error" message const spy = vi.spyOn(console, "error") spy.mockImplementation(() => {}) - expect(() => renderHook(() => useRepo())).toThrow( + expect(() => render( {}}/>)).toThrow( /Repo was not found on RepoContext/ ) spy.mockRestore() @@ -21,7 +29,8 @@ describe("useRepo", () => { const wrapper = ({ children }) => ( ) - const { result } = renderHook(() => useRepo(), { wrapper }) - expect(result.current).toBe(repo) + const onRepo = vi.fn() + render(, { wrapper }) + expect(onRepo).toHaveBeenLastCalledWith(repo) }) })