ethereum / pytest-ethereum

MIT License
19 stars 12 forks source link

Proposal: Add Testing Contract class #8

Open fubuloubu opened 5 years ago

fubuloubu commented 5 years ago

Suggestion is to subclass web3's Contract class and add necessary functionality specifically to streamline testing. Suggested functionality includes:

fubuloubu commented 5 years ago

Log class would enable you to make easier expected results:

expected_log = contract.MyLog({'a': 123, 'b': 456})
contract.transaction()  # Emits log
assert contract.logs[-1] == expected_log
fubuloubu commented 5 years ago

Gas usage measurement would be really cool. Imagine having the gas usage per function emitted as a result from testing over the different scenarios tested (min/max/average)

fubuloubu commented 5 years ago

Stealing liberally from Waffle, this is how they do events testing:

  it('Transfer emits event', async () => {
    await expect(token.transfer(walletTo.address, 7))
      .to.emit(token, 'Transfer')
      .withArgs(wallet.address, walletTo.address, 7);
  });

maybe something like this:

receipt = token.transfer(walletTo, 7)  # handles transaction mining
assert len(receipt.logs) == 1  # Should only emit 1 log (optional)
assert receipt.logs[-1] == token.logs.Transfer(from=wallet, to=walletTo, amount=7)
# Note: compares log topic, then arg names, then (converted) arg values
pipermerriam commented 5 years ago

I love stealing!

fubuloubu commented 5 years ago

Me too!

or even:

token.transfer(walletTo, 7)  # handles transaction mining
expected = token.logs.Transfer(from=wallet, to=walletTo, amount=7)
assert expected in token.new_logs
# Note: compares log topic, then arg names, then (converted) arg values
pipermerriam commented 5 years ago

I want to point out that it's probably a no-go to inject/force our own contract class. Note that ethpm uses its own contract class to handle linking, and it's possible a user has their own contract class to make working with their contracts easier (like @property methods for getters that take no arguments).

Which points to a larger issue which is whether it's a good idea for ethpm or any library for that matter to inject their own contract class and whether we can come up with an API that doesn't suffer from these deficiencies.

fubuloubu commented 5 years ago

Yeah, I think web3py defining the most verbose and widely useful contract class is nice, but each subclass adds features or modifies different properties, which muddles the API over time over different uses. Imagine using one class's assumptions to deploy, versus another to test. It would get difficult to manage mentally over time!

I do however think that functional testing should be more highly specific and less verbose. It makes it easier to write tests quickly and definitely easier to read them. Whether this specificness rests in this plugin, or through registering one of a number of purpose-built classes that the user likes the feel of, that is up for discussion.

Imagine if ImplicitContract and ConciseContract were not part of web3py, but were optional "shortcut classes" you could install separately to streamline how you work with contracts across the board?

pipermerriam commented 5 years ago

Implicit/Concise are being removed in favor of ContractReader which doesn't expose any transact APIs and is a pure read-only interface.

And with that, I'm considering whether we just add Contract.reader as a property which users can access that API from, which naturally extends to stuff like 3rd party apis from eth-tester such as Contract.testing which might expose some test-case friendly apis.

fubuloubu commented 5 years ago

Hmm, I'd have to see how that works out. I can't picture it in my head.

My first point is test cases should be easy to write and audit intent. My second point is context-switching between testing and debugging/deploying should be easy.