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

Fix problem with a mock resolving a mocked value with Promises #192

Closed
wants to merge 2 commits into from

Conversation

jlkeesey
Copy link
Contributor

The long description is in #191 so this is short.

This fixes a problem where returning a mocked object through thenResolve() on another mocked object never resolves.

By default, all symbols except .hasOwnProperty() are stubbed even if
they are not assigned a value. This is to be able to track references to
all invocations. However, the way that Promise.resolve() works, it
checks for the presence of a .then() property and if it is there assumes
that the resolve value is a promise so it tries to get its resolved
value. But this is a mock reference so the stub does not return a
resolved value so the promise never resolves.

This fix excludes .then() and .catch() from automatically creating a
stub so that Promise will not try to resolve it, but the user can still
override the call with a when() clause.

This fix also adds "Symbol(Symbol.toPrimitive)" to the exclude list so
that by default toString() on a mock will return something other than
null.
Added test to verify that adding "Symbol(Symbol.toPrimitive)" to the
exclude list will allow toString() conversion to work properly.
@codecov-commenter
Copy link

Codecov Report

Merging #192 into master will decrease coverage by 0.55%.
The diff coverage is 20.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #192      +/-   ##
==========================================
- Coverage   93.55%   92.99%   -0.56%     
==========================================
  Files          34       34              
  Lines         652      657       +5     
  Branches       80       82       +2     
==========================================
+ Hits          610      611       +1     
- Misses         30       32       +2     
- Partials       12       14       +2     
Impacted Files Coverage Δ
src/Mock.ts 91.83% <20.00%> (-2.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6637048...4d3ce94. Read the comment docs.

@jlkeesey
Copy link
Contributor Author

I only added tests so I don't know why the coverage is down and I only added 6 lines (7 if you count the variable definition).

@jlkeesey
Copy link
Contributor Author

Something isn't working, my environment may be wrong. I'm going to close this until I can figure out what is going on.

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