Skip to content

chore: setup testing libraries and tests for App.tsx #55

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

brettkolodny
Copy link
Collaborator

@brettkolodny brettkolodny commented Aug 5, 2025

Sets up vitest and adds tests for App.tsx

Fist PR of two for #45

Copy link

vercel bot commented Aug 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 5, 2025 4:29pm

- name: Setup pnpm
uses: pnpm/action-setup@v4
with:
version: 8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use 9 in dogfood (probably soon to be 10)

test: {
globals: true,
environment: 'happy-dom',
// setupFiles: ['./src/test/setup.ts'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// setupFiles: ['./src/test/setup.ts'],

Comment on lines +14 to +24
coverage: {
provider: 'v8',
reporter: ['text', 'json', 'html'],
exclude: [
'node_modules/',
'src/test/',
'**/*.d.ts',
'**/*.config.*',
'src/vite-env.d.ts'
]
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are we doing with the coverage information? do we want to upload it somewhere?

resolve: {
alias: {
'@': path.resolve(__dirname, './src'),
"monaco-editor": "./src/__mocks__/monaco-editor.ts"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the docs recommend using test.alias rather than resolve.alias for mocks. I'm not sure that there's any mechanical different, but the separation of concerns does seem kind of nice.

https://vitest.dev/guide/mocking.html#modules

'src/vite-env.d.ts'
]
},
// Mock WebAssembly for tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

"postcss": "^8.5.3",
"tailwindcss": "3",
"typescript": "~5.8.3",
"typescript-eslint": "^8.30.1",
"vercel": "^42.3.0",
"vite": "^6.3.5",
"vite-plugin-vercel": "^9.0.7"
"vite-plugin-vercel": "^9.0.7",
"vitest": "^3.2.4"
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
}

\n


- name: Install dependencies
run: pnpm install

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: Install dependencies
run: pnpm biome check

tho eslint is also in here I guess? do we need both or can we get rid of eslint?

import { ThemeProvider } from "@/client/contexts/theme";
import attachGPUExample from "@/examples/code/attach-gpu.tf?raw";
import defaultExample from "@/examples/code/default.tf?raw";
import { initWasm } from "@/utils/wasm";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this actually work? you're importing it before the mock is set up (I always avoid mocks because this sort of thing is so confusing to me)

Comment on lines +102 to +111
it("should not show the loading modal after the wasm has been loaded", async () => {
render(<TestApp />);

await waitFor(async () => {
expect(initWasm).toHaveBeenCalled();
});

expect(screen.queryByTestId("wasm-loading-modal")).toBeNull();
expect(screen.queryByTestId("wasm-error-modal")).toBeNull();
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have more confidence in this test if it was merged with "should show the loading modal while the wasm module is loading"

this is all kind of one behavior. it should show when it's loading, and shouldn't when it's not. seeing that it does in fact go away after being shown is the characteristic we'd really like to see here, but all this checks is that it's not their now. it might've never been, the test never asserts that.

@@ -0,0 +1,39 @@
/// <reference types="vitest" />
import { defineConfig } from 'vite'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this file is formatted differently from our usual "double quotes with semicolons"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants