-
Notifications
You must be signed in to change notification settings - Fork 82
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
Migrating from expect.js breaks assertions on exceptions #101
Comments
Thanks you so much for the repoducable bug report! Can you reveal what transformation you manually do right now to get it to work? Thanks. : ) |
Sorry for not replying right away - posting code examples from my phone turned out to be a nightmare 😁 I'd expect the following diff: - expect(someFn).withArgs('arbitrary', 'args').to.throw()
+ expect(() => someFn('arbitrary', 'args')).toThrow() This is what I went through and did for my entire codebase - a good hour of looking through a diff and correcting :P I totally understand that this might be hard to implement, but perhaps we can warn about them instead of just removing them? |
I was migrating a code base from Automattic's expect.js, and I noticed that my
.withArgs
-calls simply disappeared.Here's a repo with a reproducible demo: https://github.com/selbekk/reproduce-jest-codemods-error
Now, I realize that you COULD write an inline function that would pass these arguments to the function, and that's what I've been doing manually now - but it shouldn't just remove this - at the very least it should warn that I need to fix some stuff manually. When searching through the code for this project, I didn't even find a mention of
withArgs
- even though it's in the documented API.Sorry I can't fix this with a PR - I need to sit down and learn codemods and ASTs first, I guess =) But at least there's a reproducible demo you can hack away at!
Please let me know if there's anything I can do to help!
The text was updated successfully, but these errors were encountered: