ethereum / beacon_chain

MIT License
209 stars 65 forks source link

Refactor and write tests for Simple Serialization (SSZ) #100

Closed hwwhww closed 5 years ago

hwwhww commented 5 years ago

What's wrong?

Currently, the tests are at tests/utils/test_serialization.py, and it's using ActiveState and PartialCrosslinkRecord classes for testing. It will be nice to use some testing-only Foo classes to cover more testing cases instead of having a dependency of beacon_chain.

How can it be fixed

Write new tests at tests/ssz/test_*.py. Cover all test cases. Includes:

  1. Add tests of serialization and deserialization of every type (ref: https://github.com/sigp/lighthouse/tree/ssz-readme/ssz#serializeencode)
  2. Refactor SSZ module
p-mc2 commented 5 years ago

Basic tests moved to tests/ssz.

Beginning to (1.) write tests for missing types, will update when moved on to (2.) refactoring SSZ to avoid duplicate work in case someone else decides to work on that part while I'm working on (1.). Cheers.

p-mc2 commented 5 years ago

All of my tests have been failing for lists. Has anyone successfully tested the serialization for lists? Maybe I am wrong by assuming [3, 4, 5] --> [b'\x03', b'\x04', b'\x05']

hwwhww commented 5 years ago

Oh, sorry for that I missed it. The minimum int length is 8, so we have:

serialize([3, 4, 5], ['int8'])
b'\x00\x00\x00\x03\x03\x04\x05'
p-mc2 commented 5 years ago

Ah! That makes a lot more sense. Thanks for the assist!