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

Buried matcher support #109

Closed
wants to merge 1 commit into from

Conversation

Schoonology
Copy link
Contributor

Matchers nested within an Object or Array (a.k.a. "buried" matchers) argument are not handled correctly.

  • Regression test
  • Fix

@searls
Copy link
Member

searls commented Jul 13, 2016

just a ping that this PR is lost but not forgotten <3

@msakrejda
Copy link

Is it still not forgotten? =D

I can look at implementing the fix if you have pointers for where to start.

@searls
Copy link
Member

searls commented Sep 29, 2016

@uhoh-itsmaciek this is still outstanding and we'd definitely like to see it implemented such that the test passes. Be sure to rebase current master first!

@msakrejda
Copy link

I've rebased and added failing regression tests for .verify (which should behave the same as .when, right?) in my fork. I've also renamed the functionality to "deep" instead of "buried" because I think that's more common, but I'm happy to rename it back.

I've looked through when.coffee, but don't quite follow where the actual matcher checks take place--it looks like store/calls keeps a global stack of calls to verify and when consumes from that stack, but still not sure where the low-level logic lies. Is args-match.coffee where I should be looking?

@searls
Copy link
Member

searls commented Oct 5, 2016

  1. Matchers should definitely work the same for when and verify, as symmetry is part of the contract we set with users about how td.js works
  2. Agree, "deep" is more obvious and typical
  3. Yes, args-match.coffee is (to the best of my recollection) the place to start looking

@msakrejda
Copy link

I've updated my branch with what I think is a working version except for error messages handling. This is the failure I get:

  241 passing (426ms)
  1 failing

  1) .verify using deep matchers unsatisfied then td.verify(_this.testDouble({ value: td.matchers.isA(String):
     Error: expected 'Unsatisfied verification on test double.\n\n  Wanted:\n    - called with `({\n  value: {\n    __name: "isA(String)",\n    __matches: function (actualArg) {\n          return config.matches(matcherArgs, actualArg);\n        }\n  }\n})`.\n\n  But was actually called:\n    - called with `({value: 55})`.' to equal 'Unsatisfied verification on test double.\n\n  Wanted:\n    - called with `({ value: isA(String) })`.\n\n  But was actually called:\n    - called with `({ value: 55 })`.'

(also I apologize for my hideous and clumsy Coffeescript; I haven't worked much with it or JS for that matter)

I think I need to make changes to src/stringify/anything.coffee to do similar recursive handling, but I get seemingly unrelated failures when I try to change things there. Any ideas?

@msakrejda
Copy link

Actually at this point, I'm just going to open my own PR; I can't update this one and I think it will be easier to discuss if I have code to point to directly.

@msakrejda msakrejda mentioned this pull request Oct 11, 2016
@searls searls mentioned this pull request Mar 11, 2017
6 tasks
@searls
Copy link
Member

searls commented Mar 12, 2017

Fixed by #203 & landed in testdouble@2.0.0-pre.4

@searls searls closed this Mar 12, 2017
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.

None yet

3 participants