-
Notifications
You must be signed in to change notification settings - Fork 676
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
Update option changes toast and switch their tests to jest #6174
Conversation
import { Options } from '../../src/shared/options'; | ||
import { registerOmnisharpOptionChanges } from '../../src/omnisharp/omnisharpOptionChanges'; | ||
|
||
import * as jestLib from '@jest/globals'; |
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.
It is possible to globally import jest type definitions so you can just use 'describe' - however the functions will conflict with mocha ones. So for now we manually import the type definitions so we can alias them.
We also need to name it something not 'jest' as it will throw errors because jest is already imported (as an alias) by the framework
import * as vscode from 'vscode'; | ||
import { getVSCodeWithConfig, updateConfig } from '../../test/unitTests/fakes'; | ||
|
||
jestLib.describe('OmniSharpConfigChangeObserver', () => { |
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.
these are essentially the same tests, just migrated to jest.
comment: string | string[]; | ||
}, | ||
...args: (string | number | boolean)[] | Record<string, any>[] | ||
) => { |
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.
Yes - apparently this is how you have to implement an interface that defines overloads - define one function that handles every possible variation on the input. Either I'm missing something (extremely likely) or TS is just insane.
@@ -1,6 +1,7 @@ | |||
{ | |||
"recommendations": [ | |||
"dbaeumer.vscode-eslint", | |||
"esbenp.prettier-vscode" | |||
"esbenp.prettier-vscode", | |||
"orta.vscode-jest" |
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.
this adds the jest test explorer - which is how you run and debug tests without manually configuring launch.json
@@ -999,4 +999,16 @@ export interface vscode { | |||
sessionId: string; | |||
openExternal(target: Uri): Thenable<boolean>; | |||
}; | |||
|
|||
l10n: { |
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 still need this vscode adapter type currently-
- The old mocha tests (and the production code being tested) still require it to run.
- The new jest tests currently require it to define the fake 'vscode' to export. We can potentially replace this fake with real mocks once everything has migrated over.
However - as we migrate more tests we can slowly remove it from being referenced in production code. I've done this partially for the option observers.
pool: | ||
name: NetCore-Public | ||
demands: $(demandsName) | ||
steps: | ||
- template: azure-pipelines/test.yml | ||
|
||
- stage: Test_Omnisharp |
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.
Added a separate leg for O# vs blue here - we actually had a few unit tests for blue, but they were running incorrectly under the O# folder. I moved them to the regular tests folder and added a leg to run them in CI as well
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.
Added CI test leg looks good to me!
going to merge, will take additional feedback in a followup if any. |
It's possible to review commit at a time (a lil messy) or all at once.
This updates option toasts to reload the window (for C#), localize them, and enable them to be tested with jest.
Jest provides the following benefits:
a. This is essentially a requirement as we add more localization strings - in order for automatic generation of the json files to work, we must use the real 'vscode' library in-place and not the vscodeAdapter.
Commits: