Don't use file system for custom spoken form tests#2638
Don't use file system for custom spoken form tests#2638AndreasArvidsson wants to merge 4 commits intomainfrom
Conversation
|
No you'd need to create a new class that you use for mocking. Then you return that class in test helpers and tell it what custom spoken forms should be rather than using file system |
I understood that, but that tests now are writing to the file system so we need to do a bit more than just create a new class. We also need to update the tests and then comes the question are we just now testing our mocks? |
|
No I don't think so. The class we're stubbing out is quite small so I think all the custom spoken forms machinery that sits atop it is worth testing Tbh if you're looking for a really quick fix you could just disable these tests for Mac for now. I think I'd either do that or do the full approach I mention above |
|
I will have a look at it when I have some spare time |
|
|
||
| async getSpokenFormEntries(): Promise<SpokenFormEntry[]> { | ||
| if (this.entries == null) { | ||
| throw new NeedsInitialTalonUpdateError("FakeTalonSpokenForms"); |
There was a problem hiding this comment.
This is what the file system based implementation does if the json file doesn't exist. The custom spoken form implementation has a special case for this error where it defaults to spoken forms without showing a error.
packages/cursorless-vscode-e2e/src/suite/scopeProvider/runCustomRegexScopeInfoTest.ts
Outdated
Show resolved
Hide resolved
| const regex = "[a-zA-Z]+"; | ||
|
|
||
| const spokenFormJsonContents = { | ||
| version: 0, |
There was a problem hiding this comment.
The version is only used by the file system implementation
| const parseTreeApi = await getParseTreeApi(); | ||
|
|
||
| const { vscodeIDE, hats, fileSystem } = await createVscodeIde(context); | ||
| const isTesting = vscodeIDE.runMode === "test"; |
There was a problem hiding this comment.
We're doing this exact check in multiple places I think storing it to a variable increases readability
packages/cursorless-vscode-e2e/src/suite/scopeProvider/runCustomRegexScopeInfoTest.ts
Show resolved
Hide resolved
|
Updated with fake spoken forms |
phillco
left a comment
phillco
left a comment
There was a problem hiding this comment.
Since the sleep seems to be the thing that fixes the issue, let's split that into a smaller we can land right away, and the change in memory files as a separate we can discuss separately/later
|
Separate pull request with just the sleep |
AndreasArvidsson
changed the title
|
Closing this |
3 participants
#2538
This should help with the ci tests failing on mac
Checklist