-
Notifications
You must be signed in to change notification settings - Fork 680
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
Integration Tests - Add gulp tasks for lsp integration tests #6188
Conversation
tasks/testTasks.ts
Outdated
omnisharpTestAssetsRootPath, | ||
testAssetName, | ||
'.vscode', | ||
`omnisharp_${engine}_${testAssetName}.code-workspace` |
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.
nit - extract into local var and construct with ternary
e.g.
const workspaceFile = `omnisharp${engine === 'lsp' ? '_lsp' : ''}_${testAssetName}.code-workspace`
test/runIntegrationTests.ts
Outdated
|
||
async function main() { | ||
try { | ||
const vscodeExecutablePath = await downloadAndUnzipVSCode('stable'); |
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.
we should share this code with the O# version since this should be basically the same (even with jest). We can probably keep only the copy in the 'test' folder. We will need to switch certain variables based on which test is running (e.g. 'extensionTestsPath')
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.
Approving, assuming the intent is to add to CI in a followup PR. If not it needs to update the gulp 'test' task to include the integation task
@@ -86,24 +91,63 @@ gulp.task( | |||
gulp.series('omnisharp:jest:test', 'omnisharptest:feature', 'omnisharptest:unit', 'omnisharptest:integration:stdio') | |||
); | |||
|
|||
gulp.task('test:integration:slnWithCsproj', async () => runIntegrationTest('slnWithCsproj')); | |||
|
|||
gulp.task('test:unit', async () => { | |||
runJestTest(/unitTests.*\.ts/); | |||
}); | |||
|
|||
gulp.task('test', gulp.series('test:unit')); |
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.
In order to enable this for CI it needs to be added here I believe
Also explicitly provides workspaces for lsp vs non lsp omnisharp test paths