Skip to content
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

Basic Vitest implementation #267

Merged
merged 9 commits into from
Jul 24, 2023
Merged

Basic Vitest implementation #267

merged 9 commits into from
Jul 24, 2023

Conversation

jackburrus
Copy link
Contributor

@jackburrus jackburrus commented Jul 24, 2023

This PR implements a basic testing suite using Vitest. I wrote the first test for multiplying bigints
Screenshot 2023-07-24 at 11 04 23 AM

@vercel
Copy link

vercel bot commented Jul 24, 2023

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

Name Status Preview Comments Updated (UTC)
hyperdrive-fixed-borrow ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 6:57pm
hyperdrive-monorepo-hyperdrive-trading ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2023 6:57pm

@@ -1,3 +1,3 @@
export const supportedChainIds = [42069, 31337] as const;

export type SupportedChainId = (typeof supportedChainIds)[number];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related to this PR, but when I pulled main this file was updated. Do we need the parens here?

Copy link
Member

Choose a reason for hiding this comment

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

Not if it builds without em 👍


const vitestConfig: VitestUserConfigInterface = {
test: {
// https://vitest.dev/config
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setup the test config so we can customize it when needed.

@@ -1,3 +1,3 @@
export const supportedChainIds = [42069, 31337] as const;

export type SupportedChainId = (typeof supportedChainIds)[number];
Copy link
Member

Choose a reason for hiding this comment

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

Not if it builds without em 👍

package.json Outdated
@@ -12,6 +12,8 @@
"dev": "turbo run dev --parallel --filter=./apps/*",
"lint": "turbo run lint",
"lint:packages": "turbo run lint --filter=./packages/*",
"test": "yarn workspace @hyperdrive/core run test",
"test:run": "yarn workspace @hyperdrive/core run test:run",
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: top-level package.json shouldn't include any specific project scripts. Let's add just add the test script, and run it the same way as other scripts here, ie: turbo run test

@@ -0,0 +1,29 @@
import { multiplyBigInt } from "src/base/multiplyBigInt";
import { test, assert } from "vitest";
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: Curious why we're using assert vs expect for assertions. I couldn't find any vitest docs about assert, but there's a whole page for how to use expect. Did I miss something?

https://vitest.dev/api/expect.html#expect-assertions

Copy link
Contributor

@DannyDelott DannyDelott left a comment

Choose a reason for hiding this comment

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

Nice! looks good, couple of small comments

@jackburrus
Copy link
Contributor Author

Fixed based on the comments above. Should be able to just run yarn run test from the root of the mono repo and tests will run in the core.

@jackburrus jackburrus merged commit 8c8aa2a into main Jul 24, 2023
@jackburrus jackburrus deleted the jack-vitest-implementation branch July 24, 2023 20:36
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.

3 participants