-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Migrate e2e tests to TypeScript #7990
Conversation
e2e/__tests__/__snapshots__/emptyDescribeWithHooks.test.js.snap
Outdated
Show resolved
Hide resolved
The failing test is |
gah, that fails on master (and has been failing for a long time) as well, but the test pattern doesn't pick it up. So just ignore that failure for now 🙂 |
Hmm, I'm not sure how to debug these actually. It seems that a different version of node throws a different error. Going by circle CI output Node 6:
Node 8:
Node 10:
|
That test has been broken since August 2017 when |
Locally, these tests are failing for me. System: Edit: Disregarding the mercurial tests failing, it seems that the only failing test for me is |
Opened up #7993 for the iterator test |
e2e/Utils.ts
Outdated
@@ -83,6 +84,7 @@ export const writeFiles = ( | |||
createDirectory(path.join.apply(path, [directory].concat(filePath))); | |||
} | |||
fs.writeFileSync( | |||
// @ts-ignore |
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.
why the ignore? The code looks really convoluted (I guess it's from a time before rest params), could we do
path.resolve(directory, ...filePath, filename),
?
Same above, just do path.join(directory, ...filePath)
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.
Oh the issue here was that filename is can possibly undefined from the pop
call. It doesnt fit the type expected by path.resolve
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.
I've pushed up some changes to make the code simpler regarding your comments. I'll just continue tomorrow. It's midnight where I'm at. Happy coding :)
I hit the failing iterator test previously as well: #4630. It should be ignored for now, mind fixing the pattern? https://github.com/facebook/jest/blob/12542a27089d9fc30ce321aa0e06e71de5f45e09/jest.config.js#L57 |
Sure thing. Lemme push it up real quick then |
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.
I'm too lazy to test locally before pushing 😅 Should be good now, though.
Thanks!
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. |
This is a work in progress PR. Doing this mostly to see if the test suite will pass. The test suite on master isn't passing on my machine. Renamed the files in the
__tests__
in the e2e folder.MockStdinPlugin
,runJest
andUtils
have yet to be migrated.