Upgrade to vitest2 #530
Upgrade to vitest2 #530
Conversation
| @@ -241,7 +242,7 @@ function getTsconfigOptions(baseTsconfigPath, opts) { | |||
| * legacy: boolean, | |||
| * esmOnly?: boolean, | |||
| * customTsconfigExcludes?: string[], | |||
| * tsVersion?: "^5.5.2"|"^4.9.5", | |||
| * tsVersion?: typeof LATEST_TYPESCRIPT_DEP | "^4.9.5", | |||
There was a problem hiding this comment.
Just preventing future annoying behavior. Typescript doesn't actually check the .mjs files so everything was working here with any string but we were getting red in the IDE. This will at least prevent that mismatch.
| @@ -89,20 +89,14 @@ describe("AsyncCache", () => { | |||
| }); | |||
|
|
|||
| describe("race checks", () => { | |||
| let factoryFn: Mock<[MinimalClient, string], Promise<any>>; | |||
| let factoryFn: Mock<AsyncFactory<any, any>>; | |||
There was a problem hiding this comment.
Honestly, this is a way better type api for vitest (taking the typeof a function instead of an array of args and a return value). Alas, it required minor changes in this file
| @@ -274,10 +268,10 @@ describe("AsyncCache", () => { | |||
| i++ | |||
| ) { | |||
| if (asyncCacheSpies.get.mock.calls[i][1] === key) { | |||
| expect(asyncCacheSpies.get.mock.results[i].type).toBe( | |||
| "throw", | |||
| expect(asyncCacheSpies.get.mock.settledResults[i].type).toBe( | |||
There was a problem hiding this comment.
vitest in 1.x was very very strange wrt to how it handled functions that returned promises. In the old way, if the returned promise was resolved/rejected, the value itself was treated like the return value from the function. Otherwise it was treated as pending. Of course a function that returns a Promise could throw unrelated to that promise and you might want to test for that. So they changed the API to be far far more logical in terms of whats happening in the underlying system but that also broke all these carefully crafted tests.
settledResults is a lot like the former results except it only fills out AFTER the promises have been resolved/rejected. In almost all of our tests, this was a pure replacement but in some cases we had to change things.
| @@ -14,18 +14,18 @@ | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| import { __testSeamOnly_NotSemverStable__GeneratePackageCommand as GeneratePackageCommand } from "@osdk/foundry-sdk-generator"; | |||
There was a problem hiding this comment.
When moving the test code from a sibling to a cousin, we needed to be able to expose a test seam for GeneratePackageCommand. This name seemed scary enough for anyone that tries to use this package (no one should to be clear).
| @@ -123,7 +122,7 @@ describe("LoadObjects", () => { | |||
|
|
|||
| const emp = assertOkOrThrow(result); | |||
|
|
|||
| expectTypeOf(emp).toEqualTypeOf<{ | |||
| expectTypeOf(emp).branded.toEqualTypeOf<{ | |||
There was a problem hiding this comment.
Because these were not actually being source code verified before (see below) this wasn't correct. __apiName became a branded string awhile ago. Which means these types arent TECHNICALLY correct. .branded looses things up seemingly stripping away brands allowing this check to keep working
| readonly fullName: string | undefined; | ||
| readonly __primaryKey: number; | ||
| readonly __apiName: "Employee"; | ||
| readonly $primaryKey: number; |
There was a problem hiding this comment.
The e2e tests in foundry-sdk-generator were not being typechecked before. For some reason they were excluded from the tsconfig. Thus when we did things like add $primaryKey to the output, we didn't know we broke tests. I didn't exclude them after the move so I had to fix the tests too
| @@ -7,5 +7,8 @@ | |||
| "include": [ | |||
| "./src/**/*" | |||
| ], | |||
| "exclude": [ | |||
| "./src/generatedNoCheck/**/*" | |||
There was a problem hiding this comment.
This code that is generated just doesnt work in THIS project for type checking. It wasnt type checked before and isnt now to be consistent.
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| alias: { |
There was a problem hiding this comment.
this file basically migrated but it changed enough that git thinks its new
| @@ -419,7 +419,7 @@ function createMockWebSocketConstructor( | |||
| eventEmitter.removeEventListener.bind(eventEmitter), | |||
| ) as any, | |||
|
|
|||
| send: vi.fn((a, _b) => { | |||
| send: vi.fn((a, _b: any) => { | |||
There was a problem hiding this comment.
Why'd you need to add a type for _b?
There was a problem hiding this comment.
in the old way of the type being args and return value, there was only one way to resolve the function. But now that its typeof foo and there can be overloads, it can't infer properly what this value is because in one case its missing and in another it was defined. This was fine before, we didnt use it and i just told it about the defined value in the type but we don't have that any more.
ericanderson
force-pushed
the
ea.vitest2
branch
2 times, most recently
from
d2aab70 to
7c9cc25
Compare
ericanderson
force-pushed
the
ea.vitest2
branch
from
a235d42 to
acb7ee3
Compare
No description provided.