-
Notifications
You must be signed in to change notification settings - Fork 3.1k
chore: migrate core (autocomplete) tests to vitest #6063
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
chore: migrate core (autocomplete) tests to vitest #6063
Conversation
- also include .test.ts files for vitest config
Your cubic subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use cubic. |
✅ Deploy Preview for continuedev canceled.
|
@@ -46,7 +46,7 @@ export async function testRootPathContext( | |||
|
|||
// Copy the folder to the test directory | |||
const folderPath = path.join( | |||
__dirname, |
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.
vitest gets the actual __dirname
where the file resides while for jest __dirname is equivalent process.cwd()
for example, in this case, __dirname for vitest would be /users/myuser/continue/core/autocomplete/context/root-path-context/test
and for jest it would be /users/myuser/continue/core
Hence used process.cwd()
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.
Looks good, I added some comments regarding __dirname
and vitest config.
|
@@ -1,3 +1,4 @@ | |||
import { describe, test } from "vitest"; |
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.
Missing required 'expect' import for test assertions. While 'describe' and 'test' are imported, the test file uses assertions that require the 'expect' function from Vitest. Without importing 'expect', the test assertions will fail at runtime. The import should be modified to include 'expect': import { describe, test, expect } from "vitest";
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
😱 Found 1 issue. Time to roll up your sleeves! 😱 |
core/vitest.config.ts
Outdated
"config/yaml/LocalPlatformClient.test.ts", | ||
"autocomplete/**/*.test.ts", |
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.
As we spoke on the huddle earlier today, let's only include ["**/*.vitest.ts"]
and update test names accordingly instead.
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.
Left a comment ⬆️
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.
Looks great, thanks for making the changes!
Description
As part of our migration of tests inside
core
from jest to vitest, this PR:autocomplete
directory.related to CON-2030
Checklist
Screenshots
[ For visual changes, include screenshots. Screen recordings are particularly helpful, and appreciated! ]
Tests
[ What tests were added or updated to ensure the changes work as expected? ]