From 9b0ff117abd176a4a75233dc65819c0f13ad5c00 Mon Sep 17 00:00:00 2001 From: Leo Date: Sat, 6 Jul 2024 12:16:30 +0800 Subject: [PATCH] CI: Support uploading frontend unit test coverage. (#2772) * CI: Support uploading frontend unit test coverage. * Add make-i18n before test. * Update vitest configuration to include only .ts and .tsx files in coverage. * remove .only in test and fix the failed tests. * Add text summary. * Move vite-tsconfig-paths to dev dep. Adjust UTs. --------- Signed-off-by: ifuryst Co-authored-by: sp.wack <83104063+amanape@users.noreply.github.com> --- .github/workflows/run-unit-tests.yml | 29 +++++++ frontend/package-lock.json | 83 +++++++++++++------ frontend/package.json | 2 + .../file-explorer/FileExplorer.test.tsx | 63 +++++++++----- .../file-explorer/TreeNode.test.tsx | 32 +++---- .../src/components/file-explorer/TreeNode.tsx | 4 +- frontend/vite.config.ts | 7 +- 7 files changed, 155 insertions(+), 65 deletions(-) diff --git a/.github/workflows/run-unit-tests.yml b/.github/workflows/run-unit-tests.yml index 0940d70a66..f4c21e68e5 100644 --- a/.github/workflows/run-unit-tests.yml +++ b/.github/workflows/run-unit-tests.yml @@ -19,6 +19,35 @@ env: PERSIST_SANDBOX : "false" jobs: + fe-test: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [20] + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + + - name: Install dependencies + working-directory: ./frontend + run: npm ci + + - name: Run tests and collect coverage + working-directory: ./frontend + run: npm run test:coverage + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v4 + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + test-on-macos: name: Test on macOS runs-on: macos-12 diff --git a/frontend/package-lock.json b/frontend/package-lock.json index f1b4f2c394..546c4ec21c 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -49,6 +49,7 @@ "@types/react-syntax-highlighter": "^15.5.13", "@typescript-eslint/eslint-plugin": "^7.15.0", "@typescript-eslint/parser": "^7.15.0", + "@vitest/coverage-v8": "^1.6.0", "autoprefixer": "^10.4.19", "eslint": "^8.57.0", "eslint-config-airbnb": "^19.0.4", @@ -714,9 +715,7 @@ "version": "0.2.3", "resolved": "https://registry.npmjs.org/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz", "integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==", - "dev": true, - "optional": true, - "peer": true + "dev": true }, "node_modules/@esbuild/aix-ppc64": { "version": "0.21.5", @@ -1504,8 +1503,6 @@ "resolved": "https://registry.npmjs.org/@istanbuljs/schema/-/schema-0.1.3.tgz", "integrity": "sha512-ZXRY4jNvVgSVQ8DL3LTcakaAtXwTVUxE81hslsyD2AtoXW/wVob10HkOJ1X/pAlcI7D+2YoZKg5do8G/w6RYgA==", "dev": true, - "optional": true, - "peer": true, "engines": { "node": ">=8" } @@ -5368,6 +5365,47 @@ "vite": "^4.2.0 || ^5.0.0" } }, + "node_modules/@vitest/coverage-v8": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@vitest/coverage-v8/-/coverage-v8-1.6.0.tgz", + "integrity": "sha512-KvapcbMY/8GYIG0rlwwOKCVNRc0OL20rrhFkg/CHNzncV03TE2XWvO5w9uZYoxNiMEBacAJt3unSOiZ7svePew==", + "dev": true, + "dependencies": { + "@ampproject/remapping": "^2.2.1", + "@bcoe/v8-coverage": "^0.2.3", + "debug": "^4.3.4", + "istanbul-lib-coverage": "^3.2.2", + "istanbul-lib-report": "^3.0.1", + "istanbul-lib-source-maps": "^5.0.4", + "istanbul-reports": "^3.1.6", + "magic-string": "^0.30.5", + "magicast": "^0.3.3", + "picocolors": "^1.0.0", + "std-env": "^3.5.0", + "strip-literal": "^2.0.0", + "test-exclude": "^6.0.0" + }, + "funding": { + "url": "https://opencollective.com/vitest" + }, + "peerDependencies": { + "vitest": "1.6.0" + } + }, + "node_modules/@vitest/coverage-v8/node_modules/istanbul-lib-source-maps": { + "version": "5.0.6", + "resolved": "https://registry.npmjs.org/istanbul-lib-source-maps/-/istanbul-lib-source-maps-5.0.6.tgz", + "integrity": "sha512-yg2d+Em4KizZC5niWhQaIomgf5WlL4vOOjZ5xGCmF8SnPE/mDWWXgvRExdcpCgh9lLRRa1/fSYp2ymmbJ1pI+A==", + "dev": true, + "dependencies": { + "@jridgewell/trace-mapping": "^0.3.23", + "debug": "^4.1.1", + "istanbul-lib-coverage": "^3.0.0" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/@vitest/expect": { "version": "1.6.0", "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-1.6.0.tgz", @@ -8752,9 +8790,7 @@ "version": "2.0.2", "resolved": "https://registry.npmjs.org/html-escaper/-/html-escaper-2.0.2.tgz", "integrity": "sha512-H2iMtd0I4Mt5eYiapRdIDjp+XzelXQ0tFE4JS7YFwFevXXMmOp9myNrUvCg0D6ws8iqkRPBfKHgbwig1SmlLfg==", - "dev": true, - "optional": true, - "peer": true + "dev": true }, "node_modules/html-parse-stringify": { "version": "3.0.1", @@ -9489,8 +9525,6 @@ "resolved": "https://registry.npmjs.org/istanbul-lib-coverage/-/istanbul-lib-coverage-3.2.2.tgz", "integrity": "sha512-O8dpsF+r0WV/8MNRKfnmrtCWhuKjxrq2w+jpzBL5UZKTi2LeVWnWOmWRxFlesJONmc+wLAGvKQZEOanko0LFTg==", "dev": true, - "optional": true, - "peer": true, "engines": { "node": ">=8" } @@ -9518,8 +9552,6 @@ "resolved": "https://registry.npmjs.org/istanbul-lib-report/-/istanbul-lib-report-3.0.1.tgz", "integrity": "sha512-GCfE1mtsHGOELCU8e/Z7YWzpmybrx/+dSTfLrvY8qRmaY6zXTKWn6WQIjaAFw069icm6GVMNkgu0NzI4iPZUNw==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "istanbul-lib-coverage": "^3.0.0", "make-dir": "^4.0.0", @@ -9550,8 +9582,6 @@ "resolved": "https://registry.npmjs.org/istanbul-reports/-/istanbul-reports-3.1.7.tgz", "integrity": "sha512-BewmUXImeuRk2YY0PVbxgKAysvhRPUQE0h5QRM++nVWyubKGV0l8qQ5op8+B2DOmwSe63Jivj0BjkPQVf8fP5g==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "html-escaper": "^2.0.0", "istanbul-lib-report": "^3.0.0" @@ -11214,13 +11244,22 @@ "@jridgewell/sourcemap-codec": "^1.4.15" } }, + "node_modules/magicast": { + "version": "0.3.4", + "resolved": "https://registry.npmjs.org/magicast/-/magicast-0.3.4.tgz", + "integrity": "sha512-TyDF/Pn36bBji9rWKHlZe+PZb6Mx5V8IHCSxk7X4aljM4e/vyDvZZYwHewdVaqiA0nb3ghfHU/6AUpDxWoER2Q==", + "dev": true, + "dependencies": { + "@babel/parser": "^7.24.4", + "@babel/types": "^7.24.0", + "source-map-js": "^1.2.0" + } + }, "node_modules/make-dir": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-4.0.0.tgz", "integrity": "sha512-hXdUTZYIVOt1Ex//jAQi+wTZZpUpwBj/0QsOzqegb3rGMMeJiSEu5xLHnYfBrRV4RH2+OCSOO95Is/7x1WJ4bw==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "semver": "^7.5.3" }, @@ -14435,8 +14474,6 @@ "resolved": "https://registry.npmjs.org/test-exclude/-/test-exclude-6.0.0.tgz", "integrity": "sha512-cAGWPIyOHU6zlmg88jwm7VRyXnMN7iV68OGAbYDk/Mh/xC/pzVPlQtY6ngoIH/5/tciuhGfvESU8GrHrcxD56w==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "@istanbuljs/schema": "^0.1.2", "glob": "^7.1.4", @@ -14451,8 +14488,6 @@ "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -14463,8 +14498,6 @@ "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz", "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==", "dev": true, - "optional": true, - "peer": true, "dependencies": { "brace-expansion": "^1.1.7" }, @@ -14611,9 +14644,9 @@ "integrity": "sha512-Y/arvbn+rrz3JCKl9C4kVNfTfSm2/mEp5FSz5EsZSANGPSlQrpRI5M4PKF+mJnE52jOO90PnPSc3Ur3bTQw0gA==" }, "node_modules/tsconfck": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/tsconfck/-/tsconfck-3.0.3.tgz", - "integrity": "sha512-4t0noZX9t6GcPTfBAbIbbIU4pfpCwh0ueq3S4O/5qXI1VwK1outmxhe9dOiEWqMz3MW2LKgDTpqWV+37IWuVbA==", + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/tsconfck/-/tsconfck-3.1.1.tgz", + "integrity": "sha512-00eoI6WY57SvZEVjm13stEVE90VkEdJAFGgpFLTsZbJyW/LwFQ7uQxJHWpZ2hzSWgCPKc9AnBnNP+0X7o3hAmQ==", "dev": true, "bin": { "tsconfck": "bin/tsconfck.js" diff --git a/frontend/package.json b/frontend/package.json index 8878cf20f9..6c4a3a448f 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -40,6 +40,7 @@ "start": "npm run make-i18n && vite", "build": "npm run make-i18n && tsc && vite build", "test": "vitest run", + "test:coverage": "npm run make-i18n && vitest run --coverage", "dev_wsl": "VITE_WATCH_USE_POLLING=true vite", "preview": "vite preview", "make-i18n": "node scripts/make-i18n-translations.cjs", @@ -71,6 +72,7 @@ "@types/react-syntax-highlighter": "^15.5.13", "@typescript-eslint/eslint-plugin": "^7.15.0", "@typescript-eslint/parser": "^7.15.0", + "@vitest/coverage-v8": "^1.6.0", "autoprefixer": "^10.4.19", "eslint": "^8.57.0", "eslint-config-airbnb": "^19.0.4", diff --git a/frontend/src/components/file-explorer/FileExplorer.test.tsx b/frontend/src/components/file-explorer/FileExplorer.test.tsx index e26e74deb8..3cd1247779 100644 --- a/frontend/src/components/file-explorer/FileExplorer.test.tsx +++ b/frontend/src/components/file-explorer/FileExplorer.test.tsx @@ -30,18 +30,24 @@ describe("FileExplorer", () => { }); it("should get the workspace directory", async () => { - const { getByText } = renderWithProviders(); + const { getByText } = renderWithProviders(, { + preloadedState: { + agent: { + curAgentState: AgentState.RUNNING, + }, + }, + }); await waitFor(() => { expect(getByText("folder1")).toBeInTheDocument(); - expect(getByText("file2.ts")).toBeInTheDocument(); + expect(getByText("file1.ts")).toBeInTheDocument(); }); - expect(listFiles).toHaveBeenCalledTimes(2); // once for root, once for folder1 + expect(listFiles).toHaveBeenCalledTimes(1); // once for root }); it.todo("should render an empty workspace"); - it.only("should refetch the workspace when clicking the refresh button", async () => { + it("should refetch the workspace when clicking the refresh button", async () => { const { getByText, getByTestId } = renderWithProviders(, { preloadedState: { agent: { @@ -53,61 +59,74 @@ describe("FileExplorer", () => { expect(getByText("folder1")).toBeInTheDocument(); expect(getByText("file1.ts")).toBeInTheDocument(); }); - expect(listFiles).toHaveBeenCalledTimes(1); // once for root, once for folder 1 + expect(listFiles).toHaveBeenCalledTimes(1); // once for root - // The 'await' keyword is required here to avoid a warning during test runs await act(async () => { await userEvent.click(getByTestId("refresh")); }); await waitFor(() => { - expect(listFiles).toHaveBeenCalledTimes(2); // 2 from initial render, 2 from refresh button + expect(listFiles).toHaveBeenCalledTimes(2); // once for root, once for refresh button }); }); it("should toggle the explorer visibility when clicking the close button", async () => { const { getByTestId, getByText, queryByText } = renderWithProviders( , + { + preloadedState: { + agent: { + curAgentState: AgentState.RUNNING, + }, + }, + }, ); await waitFor(() => { expect(getByText("folder1")).toBeInTheDocument(); }); - act(() => { - userEvent.click(getByTestId("toggle")); + await act(async () => { + await userEvent.click(getByTestId("toggle")); }); - // it should be hidden rather than removed from the DOM expect(queryByText("folder1")).toBeInTheDocument(); expect(queryByText("folder1")).not.toBeVisible(); }); it("should upload files", async () => { // TODO: Improve this test by passing expected argument to `uploadFiles` - const { getByTestId } = renderWithProviders(); + const { findByTestId } = renderWithProviders(, { + preloadedState: { + agent: { + curAgentState: AgentState.RUNNING, + }, + }, + }); + const file = new File([""], "file-name"); const file2 = new File([""], "file-name-2"); - const uploadFileInput = getByTestId("file-input"); + const uploadFileInput = await findByTestId("file-input"); - // The 'await' keyword is required here to avoid a warning during test runs - await act(() => { - userEvent.upload(uploadFileInput, file); + await act(async () => { + await userEvent.upload(uploadFileInput, file); }); expect(uploadFiles).toHaveBeenCalledOnce(); expect(listFiles).toHaveBeenCalled(); - const uploadDirInput = getByTestId("file-input"); + const uploadDirInput = await findByTestId("file-input"); // The 'await' keyword is required here to avoid a warning during test runs - await act(() => { - userEvent.upload(uploadDirInput, [file, file2]); + await act(async () => { + await userEvent.upload(uploadDirInput, [file, file2]); }); - expect(uploadFiles).toHaveBeenCalledTimes(2); - expect(listFiles).toHaveBeenCalled(); + await waitFor(() => { + expect(uploadFiles).toHaveBeenCalledTimes(2); + expect(listFiles).toHaveBeenCalled(); + }); }); it.skip("should upload files when dragging them to the explorer", () => { @@ -126,8 +145,8 @@ describe("FileExplorer", () => { const uploadFileInput = getByTestId("file-input"); const file = new File([""], "test"); - act(() => { - userEvent.upload(uploadFileInput, file); + await act(async () => { + await userEvent.upload(uploadFileInput, file); }); expect(uploadFiles).rejects.toThrow(); diff --git a/frontend/src/components/file-explorer/TreeNode.test.tsx b/frontend/src/components/file-explorer/TreeNode.test.tsx index 5172d68903..ab6f0625f9 100644 --- a/frontend/src/components/file-explorer/TreeNode.test.tsx +++ b/frontend/src/components/file-explorer/TreeNode.test.tsx @@ -50,12 +50,12 @@ describe("TreeNode", () => { expect(await findByText("folder1")).toBeInTheDocument(); expect(await findByText("file2.ts")).toBeInTheDocument(); - act(async () => { - userEvent.click(await findByText("folder1")); + await act(async () => { + await userEvent.click(await findByText("folder1")); }); expect(await findByText("folder1")).toBeInTheDocument(); - expect(await queryByText("file2.ts")).not.toBeInTheDocument(); + expect(queryByText("file2.ts")).not.toBeInTheDocument(); }); it("should open a folder when clicking on it", async () => { @@ -64,10 +64,10 @@ describe("TreeNode", () => { ); expect(await findByText("folder1")).toBeInTheDocument(); - expect(await queryByText("file2.ts")).not.toBeInTheDocument(); + expect(queryByText("file2.ts")).not.toBeInTheDocument(); - act(() => { - userEvent.click(getByText("folder1")); + await act(async () => { + await userEvent.click(getByText("folder1")); }); expect(listFiles).toHaveBeenCalledWith("/folder1/"); @@ -75,16 +75,16 @@ describe("TreeNode", () => { expect(await findByText("file2.ts")).toBeInTheDocument(); }); - it.only("should call a fn and return the full path of a file when clicking on it", () => { + it("should call a fn and return the full path of a file when clicking on it", async () => { const { getByText } = renderWithProviders( , ); - act(() => { - userEvent.click(getByText("file2.ts")); + await act(async () => { + await userEvent.click(getByText("file2.ts")); }); - waitFor(() => { + await waitFor(() => { expect(selectFile).toHaveBeenCalledWith("/folder1/file2.ts"); }); }); @@ -98,10 +98,10 @@ describe("TreeNode", () => { expect(await findByText("file1.ts")).toBeInTheDocument(); expect(await findByText("folder1")).toBeInTheDocument(); - expect(await queryByText("file2.ts")).not.toBeInTheDocument(); + expect(queryByText("file2.ts")).not.toBeInTheDocument(); - act(() => { - userEvent.click(getByText("folder1")); + await act(async () => { + await userEvent.click(getByText("folder1")); }); expect(listFiles).toHaveBeenCalledWith("folder1/"); @@ -117,10 +117,10 @@ describe("TreeNode", () => { ); expect(await findByText("folder1")).toBeInTheDocument(); - expect(await queryByText("file2.ts")).not.toBeInTheDocument(); + expect(queryByText("file2.ts")).not.toBeInTheDocument(); - act(() => { - userEvent.click(getByText("folder1")); + await act(async () => { + await userEvent.click(getByText("folder1")); }); expect(listFiles).toHaveBeenCalledWith("/folder1/"); diff --git a/frontend/src/components/file-explorer/TreeNode.tsx b/frontend/src/components/file-explorer/TreeNode.tsx index 67fe761080..31242f5da1 100644 --- a/frontend/src/components/file-explorer/TreeNode.tsx +++ b/frontend/src/components/file-explorer/TreeNode.tsx @@ -58,7 +58,9 @@ function TreeNode({ path, defaultOpen = false }: TreeNodeProps) { }; React.useEffect(() => { - refreshChildren(); + (async () => { + await refreshChildren(); + })(); }, [refreshID, isOpen]); const handleClick = async () => { diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index e15a04a6b6..15a67d8f91 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -1,8 +1,8 @@ /// import { defineConfig, loadEnv } from "vite"; import react from "@vitejs/plugin-react"; +// eslint-disable-next-line import/no-extraneous-dependencies import viteTsconfigPaths from "vite-tsconfig-paths"; -import path from "path"; export default defineConfig(({ mode }) => { const { @@ -62,6 +62,11 @@ export default defineConfig(({ mode }) => { environment: "jsdom", globals: true, setupFiles: ["vitest.setup.ts"], + coverage: { + reporter: ["text", "json", "html", "lcov", "text-summary"], + reportsDirectory: "coverage", + include: ["src/**/*.{ts,tsx}"], + }, }, }; });