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

Ensure captors only fire when invocation satisfies when/verify #311

Merged
merged 8 commits into from
Dec 20, 2017

Conversation

searls
Copy link
Member

@searls searls commented Dec 20, 2017

@jamestalmage made a good catch in #308 that captors could be used with td.when but it was undocumented and it would also capture values when the rest of the rehearsal did not match the actual args. This was effectively a requirements miss, but because our goal is to have td.when and td.verify to behave as symmetrically as possible, it's something we needed to fix.

James implemented a fix in #310, but I had three quibbles with it: (a) it broadened awareness of "capturing" to matchers themselves, even though captors are a one-off subset, (b) it expanded the private namespace claimed by matchers with __capture, and (c) it didn't verify that an argument was a matcher before invoking __capture.

  • Commit James' failing tests
  • implement an afterSatisfaction hook to the matcher creation, which only fires after all args match on an invocation or verification
  • document the new hook
  • make the new behavior work for the in-place rewrite that's still (slowly) underway; right now that means https://github.com/testdouble/testdouble.js/blob/master/src/when/index.js needs to be analyzed and updated with this responsibility

Fixes #310
Fixes #308

@searls
Copy link
Member Author

searls commented Dec 20, 2017

By the way, I've intentionally chosen not to document this td.when support for argument captors right now. If you're simply capturing a value (as opposed to a function), I think argThat is usually more succinct & local. If you're capturing a function, then td.callback is often more appropriate. I can't think of a place where I'd explicitly recommend it over other constructs provided by the lib

@searls
Copy link
Member Author

searls commented Dec 20, 2017

Also dependency CI appears to be broken. Disabling

@searls searls merged commit 18cf0e2 into master Dec 20, 2017
@searls
Copy link
Member Author

searls commented Dec 20, 2017

Landed in 3.2.7

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.

Disallow captors in td.when(...)
2 participants