Skip to content
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

chai-should: don't update member expressions inside expect calls #503

Conversation

kristerkari
Copy link
Contributor

Hello again,

I noticed that the chai-should codemod did fail to properly transform expect(fetchMock.called("/url")). (docs: http://www.wheresrhys.co.uk/fetch-mock/)

I thought that a good fix would be not to update the matchers if they appear inside an expect call. This did not break any of the new or existing test cases.

I'm not sure if there is a better way than what I used in the isInsideExpectCall function that I added.

input:

expect(fetchMock.called("/url")).to.equal(true);

expected result:

expect(fetchMock.called("/url")).toBe(true);

actual result before the fix:

expect(fetchMock.toHaveBeenCalled()).toBe(true);

ping @skovhus

@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.10 ⚠️

Comparison is base (5c01c62) 92.47% compared to head (9d3e89b) 92.38%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #503      +/-   ##
==========================================
- Coverage   92.47%   92.38%   -0.10%     
==========================================
  Files          26       26              
  Lines        1941     1944       +3     
  Branches      404      405       +1     
==========================================
+ Hits         1795     1796       +1     
- Misses        101      102       +1     
- Partials       45       46       +1     
Impacted Files Coverage Δ
src/transformers/chai-should.ts 94.54% <100.00%> (-0.57%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@skovhus skovhus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@skovhus skovhus merged commit 1ae676f into skovhus:main Mar 22, 2023
@skovhus
Copy link
Owner

skovhus commented Mar 22, 2023

Let me know if you wait on a release of these nice PRs. I assume you already made the required adjustments to the codebase you converted, otherwise let me know.

@kristerkari kristerkari deleted the chai-should-dont-update-member-expressions-inside-expect-calls branch March 22, 2023 21:00
@kristerkari
Copy link
Contributor Author

kristerkari commented Mar 22, 2023

@skovhus a new release would be nice since I haven't actually transformed the project to Jest yet, just having a look at getting the codemod to a good state.

I was still thinking of contributing support for sinon.assert (https://sinonjs.org/releases/latest/assertions/), but that is probably going to take a bit of time to implement, and could probably be then released in a separate version.

@skovhus
Copy link
Owner

skovhus commented Mar 22, 2023

@kristerkari makes sense. Version 0.31.0 has just been released. For some reason the deployment of 0.30.0 never came through, so there was quite a lot of improvements...

FYI I've switched from yarn classic to pnpm and we are now using GitHub actions instead of CircleCI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants