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

expect(...).to.be.called #229

Closed
marekkirejczyk opened this issue Apr 21, 2020 · 5 comments · Fixed by #233
Closed

expect(...).to.be.called #229

marekkirejczyk opened this issue Apr 21, 2020 · 5 comments · Fixed by #233

Comments

@marekkirejczyk
Copy link
Contributor

marekkirejczyk commented Apr 21, 2020

Add matchers for checking if the mock contract function was called with specific arguments.

Example:

it('calls balanceOf', async () => {
  await tokenMock.balanceOf.returns('1000000')); 
  await someContract.check();
  await expect(tokenMock.balanceOf).to.be.called
});
This was referenced Apr 21, 2020
@marekkirejczyk marekkirejczyk changed the title Expect to be called on matchers expect(...).to.be.called Apr 21, 2020
@sz-piotr
Copy link
Contributor

It is important to not interfere with https://www.npmjs.com/package/sinon-chai

@sz-piotr
Copy link
Contributor

This is what we came up with @Jozwiaczek

expect(contract).to.be.ethCalled
expect([contract, 'function']).to.be.ethCalled
expect([contract, 'function']).to.be.ethCalledWith(arg1, arg2)

@marekkirejczyk
Copy link
Contributor Author

marekkirejczyk commented Apr 23, 2020

Originally proposed syntax is not possible to implement.
Below we present three major syntax proposals we came up with on the call with @sz-piotr .
We will vote on the syntax internally on Ethworks slack to see what are our devs preference (hoping it will extrapolate to a wider audience)

1) 
expect('functionName').to.be.calledOn(contract)
expect('functionName').to.be.calledOn(contract).withArgs(args)

2) 
expect(contract).to.be.ethCalled
expect([contract, 'function']).to.be.ethCalled
expect([contract, 'function']).to.be.ethCalledWith(arg1, arg2)

3) 
expect(contract).to.be.calledWith('functionName')
expect(contract).to.be.calledWith('functionName', ...args)
expect(contract).to.be.calledWith('functionName').withArgs(args)

@marekkirejczyk
Copy link
Contributor Author

marekkirejczyk commented Apr 23, 2020

The results are:
Option 1) - 6 votes
Option 2) - 1 vote
Option 3) - 3 votes

@Jozwiaczek Jozwiaczek linked a pull request Apr 23, 2020 that will close this issue
@marekkirejczyk
Copy link
Contributor Author

marekkirejczyk commented Apr 24, 2020

Voted Option 1) still is impossible to implement (name collision with sinon-chai, .not. impossible to implement), modified proposals below.

1)
expect('functionName').to.be
  .calledOnContract(contract)
expect('functionName').not.to.be.calledOnContract(contract)
expect('functionName').to.be.calledOnContract(contract).withArgs(args)
expect('functionName')
  .not.to.be.calledOnContract(contract)
  .withArgs(args)

2) 
expect('functionName').to.be.calledOnContract(contract)
expect('functionName').not.to.be.calledOnContract(contract)
expect('functionName').to.be.calledOnContractWith(contract, ...)
expect('functionName').not.to.be.calledOnContractWith(contract, ...)

3) Other ideas:
expect('functionName').to.be.triggeredOn(contract)
expect('functionName').to.be.sendMsgTo(contract)

On the call with @sz-piotr @Jozwiaczek we decided to implement 2).

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 a pull request may close this issue.

2 participants