Jest jasmine/ts migration #7970
Conversation
|
Heh, 295 errors 😅 Could you ping me when this is getting more ready for review (or if you have any questions)? |
|
@SimenB Haha. It is not ready yet. I just wanted to try Draf PR 😄 . I will ping when it is ready |
|
BTW, I wouldn't worry too much about typing |
…tion # Conflicts: # packages/jest-jasmine2/src/jasmine/Env.js
|
@doniyor2109 since you commited |
|
@SimenB Yea, it took too long for this migration. I just finished types. However tests are failing because of wired error. Somehow babel does not transpile static fields and property fields correctly. 🤔 |
…jest-jasmine/ts_migration # Conflicts: # packages/jest-jasmine2/src/index.ts
| } | ||
| if (currentDeclarationSuite.markedTodo) { | ||
| // @ts-ignore TODO Possible error: Suite does not have todo method | ||
| suite.todo(); |
There was a problem hiding this comment.
todo does not exists on Suite. It might be error. I just ignored for now
| if (error instanceof AssertionError) { | ||
| checkIsError = false; | ||
| // @ts-ignore TODO Possible error: j$.Spec does not have expand property | ||
| message = assertionErrorMessage(error, {expand: j$.Spec.expand}); |
There was a problem hiding this comment.
This is also might be error.
There was a problem hiding this comment.
However tests are passing
There was a problem hiding this comment.
Since the tests are passing, I wouldn't worry too much about it
| @@ -6,8 +6,6 @@ | |||
| * | |||
| */ | |||
|
|
|||
| 'use strict'; | |||
|
|
|||
| describe('iterators', () => { | |||
There was a problem hiding this comment.
These tests looks like they belong in expect, don't they?
/cc @pedrottimark
There was a problem hiding this comment.
Yes, I will make sure that expect covers these test cases.
| @@ -6,8 +6,6 @@ | |||
| * | |||
| */ | |||
|
|
|||
| 'use strict'; | |||
|
|
|||
| describe('matchers', () => { | |||
There was a problem hiding this comment.
same here - seems like they should be in expect
There was a problem hiding this comment.
This one might be needed?
describe('matchers', () => {
it('proxies matchers to expect', () => {
expect(() => expect(1).toBe(2)).toThrowErrorMatchingSnapshot();
});
});There was a problem hiding this comment.
Yeah, but it doesn't test anything jasmine? I don't understand what it means by "proxies to expect" - it uses expect directly
There was a problem hiding this comment.
In other words, Jest test suite is integration test that expect works in jasmine.
There was a problem hiding this comment.
Right - if expect didn't work in jasmine, the whole test suite would fail. The advantage of using Jest to test Jest 🙂
|
This is huge and I just took a quick look. I'll trust @SimenB here. We can merge it, release an alpha and test it in FB code. If everything is good we can just release a new minor after the full TS migration. Please take into account that jest-circus will be the default in the next major, so Jasmine won't be that critical at that point. |
|
Thanks for the huge contribution @doniyor2109!! |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Migration to TS
Test plan