-
Notifications
You must be signed in to change notification settings - Fork 0
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
mrc-6021 Vitest server pt 1 #234
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## mrc-6018 #234 +/- ##
=========================================
Coverage 99.78% 99.78%
=========================================
Files 159 159
Lines 4107 4107
Branches 936 938 +2
=========================================
Hits 4098 4098
Misses 8 8
Partials 1 1 ☔ View full report in Codecov by Sentry. |
@@ -27,5 +27,5 @@ | |||
<script type="module" src="/wodin.js"></script> | |||
<link rel="stylesheet" href="/wodin.css"/> | |||
{{/hotReload}} | |||
<script async src="{{mathjaxSrc}}"></script> | |||
<script async src="{{& mathjaxSrc}}"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to escape this special character in mathjaxSrc too (its a URL)
const { mockSessionStore, mockGetSessionStore } = vi.hoisted(() => { | ||
const mockSessionStore = { | ||
getSessionIdFromFriendlyId: vi.fn().mockImplementation(() => { return sessionIdFromFriendlyId; }) | ||
}; | ||
const mockGetSessionStore = vi.fn().mockReturnValue(mockSessionStore); | ||
return { | ||
mockSessionStore, | ||
mockGetSessionStore | ||
} | ||
}) | ||
vi.mock("../../src/db/sessionStore", () => { return { getSessionStore: mockGetSessionStore }; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to explain this pattern in a couple of places in this PR and future ones:
vi.mock
works almost the same as jest.mock
however it cannot capture external variables except for those that have been hoisted through vi.hoisted
so vi.hoisted
expressions are always run first so are guaranteed to exist for the mock, so most of it is the same, just any variable (such as a mocked method that you want to test) that is used in the tests and the mock must be hoisted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm are you sure?? ive never had to hoist like this when using vitest.. if your mock starts with mock it should auto hoist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM... just left a small comment about hoisting
const { mockSessionStore, mockGetSessionStore } = vi.hoisted(() => { | ||
const mockSessionStore = { | ||
getSessionIdFromFriendlyId: vi.fn().mockImplementation(() => { return sessionIdFromFriendlyId; }) | ||
}; | ||
const mockGetSessionStore = vi.fn().mockReturnValue(mockSessionStore); | ||
return { | ||
mockSessionStore, | ||
mockGetSessionStore | ||
} | ||
}) | ||
vi.mock("../../src/db/sessionStore", () => { return { getSessionStore: mockGetSessionStore }; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm are you sure?? ive never had to hoist like this when using vitest.. if your mock starts with mock it should auto hoist
This is the first batch of tests. The general change is
jest
->vi
the other points of interests are highlighted by comments, CI should still not be passing but the relevant tests should be passing, everything will be passing by the next PR