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

Carver/low precision decimal test #27

Merged
merged 3 commits into from
Feb 6, 2018

Conversation

carver
Copy link
Collaborator

@carver carver commented Feb 6, 2018

What was wrong?

A version bump failed, that's weird:
https://travis-ci.org/ethereum/eth-abi/jobs/337782324

Even weirder, a travis test on the same commit succeeded:
https://travis-ci.org/ethereum/eth-abi/jobs/337782341

Oh, it's a hypothesis test, so we had to get lucky on finding the example. Turns out the failure was related to building the expected value in the test, not in the project source.

How was it fixed?

  • add the failing example to make sure the case is always covered
  • set precision higher when building expected value
  • too many magic numbers: moved to centralized decimal.Context definition
  • grep'd for all Decimal usages to put them in the abi_decimal_context

Cute Animal Picture

Cute animal picture

Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations on unravelling this one. Those hypothesis tests are somewhat insane to parse.

@carver carver merged commit fdbc34e into master Feb 6, 2018
@carver carver deleted the carver/low-precision-decimal-test branch February 6, 2018 19:54
carver added a commit to carver/eth-abi that referenced this pull request Jan 24, 2020
Skip venv in template filler & print progress
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