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

Add spy-arg-matcher for more complex matching of spy arguments #234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Fuco1
Copy link
Contributor

@Fuco1 Fuco1 commented Mar 14, 2023

@snogge
Copy link
Collaborator

snogge commented Mar 21, 2023

I like the idea, but I don't like the syntax. It would be cool if we could use pcase patterns for this. That would probably require a new matcher that receives the argument patterns in a list. The hardest part is probably to figure out a good name for the new matcher 😃

@Fuco1
Copy link
Contributor Author

Fuco1 commented Mar 22, 2023

I would argue against pcase or at least that we provide other syntax. Many people including myself find the pcase syntax quite complicated to read.

That would probably require a new matcher that receives the argument patterns in a list.

Something like adding :args-to-match-pattern ? I kept the existing matcher for "backward" compatibility so it's completely opt-in. A new matcher would also achieve that of course.

@snogge
Copy link
Collaborator

snogge commented Mar 22, 2023

The current :to-have-been-called-with expects the arguments as &rest:

(expect 'foo :to-have-been-called-with 1 2 3)

That makes it difficult to make a pattern matching matcher, how do I see that

(expect 'foo :to-have-been-called-with 1 'predfunc 3)

is supposed to use predfunc as a predicate instead of expecting that symbol as an argument? Hard to change in a backward compatible way.
A new matcher could use

(expect 'foo :to-have-been-called-matching '(1 2 3))
(expect 'foo :to-have-been-called-matching 'predfunc)

and easily see that predfunc shall be used as a matching predicate. If the single argument to :to-have-been-called-matching is assumed to be a pattern (when it is a list) we could do more with less verbose syntax.

I agree that pcase has an ... interesting pattern language, but it is there to use which is a plus.

@Fuco1
Copy link
Contributor Author

Fuco1 commented Mar 22, 2023

Well, you wouldn't say 'predfunc you would say (spy-arg-matcher #'predfunc). That's why I wrap the arguments in the spy-arg-matcher wrapper. If you pass anything which is not this structure, it is taken as a value to equal with. Otherwise the matcher's predicate is called on that position's argument. The mockery docs explain this in more detail, it's basically the same API.

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