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

Support ambiguous property types on mocked interfaces #248

Merged
merged 10 commits into from
Mar 8, 2024

Conversation

notanengineercom
Copy link
Collaborator

@notanengineercom notanengineercom commented Oct 15, 2022

Fixes #178

This allows mocking callable interfaces. See details in the linked issue.
This also solves a known issue where ambiguous property types of an interface couldn't be asserted (a property that can called as a method and a property).
The implementation is not ideal, but it works and avoids suppressing a valid expectation.
By deferring the exception we can cover this case as well, with the drawback that it escapes the context scope and bubbles up to the main context, resulting in a uncaughtException.
E.g

interface Library {
  subSection: AmbiguousSection
}

interface AmbiguousSection {
  (): string
  subMethod: () => string
}


const lib = Substitute.for<Library>()
lib.subSection().returns('subSection as method')
lib.subSection.returns({ subMethod: () => 'subSection as property' } as AmbiguousSection)

lib.subSection()
lib.subSection()
lib.subSection.subMethod()

// Assertion logic before
lib.received(2).subSection() // This would fail with a Call count mismatch exception, because when executing the assertion, the proxy can't know if we are referring to a property or a method.
// The proxy would run like:
// 1. I'm in a assertion, waiting to see what property is called
// 2. I received subSection, let me run the assertion
// 3. The assertion finds that there have been 1 property call to lib.subSection, but the assertion expects 2
// 4. It throws SubstituteException with the due to a call count mismatch

// Assertion logic now
lib.received(2).subSection() // This would succeed because of the deferred exception.
// The proxy would run like:
// 1. I'm in a assertion, waiting to see what property is called
// 2. I received subSection, let me run the assertion
// 3. The assertion finds that there has been 1 property call to lib.subSection, the assertion expects 2 but there has been also method calls to lib.subSection. Schedule the call count mismatch exception.
// 4. I'm getting a method call to myself (subSection). Let me run the assertion
// 5. The assertion cancels the scheduled exception. It finds that there have been 2 method calls to lib.subSection and the assertion expects 2. The assertion ends

@notanengineercom notanengineercom added wip Work in progress - should not merge hacktoberfest-accepted ✨v2 release labels Oct 15, 2022
@notanengineercom notanengineercom self-assigned this Oct 15, 2022
@ffMathy
Copy link
Owner

ffMathy commented Oct 15, 2022

Very interesting! 😍 Feel free to merge when you feel like it's ready.

@notanengineercom
Copy link
Collaborator Author

I'm aware of the failing test - currently trying to think how to solve it.
The main issue is, that the assertion context doesn't know yet if the object member to assert is a method or a property :/

@notanengineercom notanengineercom changed the title Support callable interfaces Support ambiguous property types on mocked interfaces Mar 7, 2024
@notanengineercom
Copy link
Collaborator Author

I added a folder to verify and test the compatibility with different test runners. I wanted to make sure that the new uncaught exception gets reported correctly in different test runners - I added checks for the most popular ones.
This is how it would look like:

Ava

Screenshot 2024-03-08 at 13 59 49

Jest

Screenshot 2024-03-08 at 14 00 34

Mocha

Screenshot 2024-03-08 at 14 01 29

Node.js

Screenshot 2024-03-08 at 14 04 27

Vitest

Screenshot 2024-03-08 at 14 05 03

@ffMathy

@notanengineercom
Copy link
Collaborator Author

This is the test output inside the compatibility-checks folder

npm test               

> [email protected] test
> node --require tsx/cjs --test test/**

▶ Verifies test runner compatibility
  ▶ ava
    ✔ reports the uncaught Substitute exception (963.726083ms)
  ▶ ava (964.909625ms)

  ▶ jest
    ✔ reports the uncaught Substitute exception (1994.183541ms)
  ▶ jest (1994.437125ms)

  ▶ mocha
    ✔ reports the uncaught Substitute exception (476.041541ms)
  ▶ mocha (476.190667ms)

  ▶ nodejs
    ✔ reports the uncaught Substitute exception (419.588625ms)
  ▶ nodejs (419.755291ms)

  ▶ vitest
    ✔ reports the uncaught Substitute exception (811.546333ms)
  ▶ vitest (811.702708ms)

▶ Verifies test runner compatibility (4667.739583ms)

ℹ tests 5
ℹ suites 6
ℹ pass 5
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 4740.15625

@ffMathy
Copy link
Owner

ffMathy commented Mar 8, 2024

Another great PR! I think I'll have mine ready soon too, and then we can ship v2.0! 😍

@ffMathy ffMathy merged commit 1e1f330 into master Mar 8, 2024
6 checks passed
@ffMathy ffMathy deleted the callable-interface-support branch March 8, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted ✨v2 release wip Work in progress - should not merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot mock callable interface
2 participants