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

Reconsider certain uses of property testing #54

Open
davesque opened this issue Apr 17, 2018 · 4 comments
Open

Reconsider certain uses of property testing #54

davesque opened this issue Apr 17, 2018 · 4 comments

Comments

@davesque
Copy link
Contributor

davesque commented Apr 17, 2018

What was wrong?

Though there is clear value in using property testing for certain kinds of tests (such as end-to-end encoding tests), I've recently had some doubts about some other ways we're using property testing.

For example:
https://github.com/ethereum/eth-abi/blob/master/tests/encoding/test_encoder_properties.py#L327-L395

To a large extent, that property test and others like it seem to be testing second implementations of functionality against the original implementations. This doesn't provide much value since the original implementation could have a flaw which wouldn't be revealed by testing it against a second implementation that uses the same logic.

How can it be fixed?

It seems like we should replace these kinds of property tests with traditional tests that check for correct behavior given specific inputs.

@carver
Copy link
Collaborator

carver commented Apr 17, 2018

No objection here. I find it very difficult to parse those tests for correctness, as is.

@davesque
Copy link
Contributor Author

Thoughts on this @pipermerriam ?

@pipermerriam
Copy link
Member

Agreed. Writing all of those tests was largely an experiment and I'd say it's failed as 1) they are very hard to read, and 2) they are effectively a second implementation of the logic (like you said).

@pipermerriam
Copy link
Member

Simplifying them to something like the following might make a lot of sense:

  • if it fails decoding: no assertions (unless we can think of good ones)
  • assertions that numeric values are strictly positive for unsigned types.
  • maybe assertions that decoded values are negative if we can reliably look at the right bit.
  • assertions that numeric values are in the appropriate range for the given type.
  • assertions that the value is an integer for types which should only yield integers.
  • assertions that the resulting type is of the appropriate type for the decoder.
  • assertions that on encoding the resulting value is the right length for any fixed length encoders.

pacrob pushed a commit to pacrob/eth-abi that referenced this issue Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants