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

Improve Codec Coverage #391

Closed
ansermino opened this issue Nov 6, 2019 · 1 comment · Fixed by #620
Closed

Improve Codec Coverage #391

ansermino opened this issue Nov 6, 2019 · 1 comment · Fixed by #620
Assignees

Comments

@ansermino
Copy link
Member

There seems to be some obvious cases missing from the codec test suite. Notably the following lines are never hit, some of which may be removable:

Encoding [32]byte:

gossamer/codec/encode.go

Lines 49 to 50 in 8fefce8

case [32]byte:
n, err = se.encodeByteArray(v[:])

Encoding common.Hash (this may be removable, custom type should be implicitly handled)

gossamer/codec/encode.go

Lines 59 to 60 in 8fefce8

case common.Hash:
n, err = se.Writer.Write(v.ToBytes())

Encoding [][]byte:

gossamer/codec/encode.go

Lines 291 to 298 in 8fefce8

case [][]byte:
n, err = se.encodeInteger(uint(len(arr)))
bytesEncoded += n
for _, elem := range arr {
n, err = se.encodeByteArray(elem)
bytesEncoded += n
}

Decoding [32]byte, common.Hash (common.Hash likely removable):

gossamer/codec/decode.go

Lines 65 to 68 in 8fefce8

case common.Hash, [32]byte:
b := make([]byte, 32)
_, err = sd.Reader.Read(b)
out = b

Decoding Interface whose value's kind is Slice or Array; kind is Struct:

gossamer/codec/decode.go

Lines 293 to 294 in 8fefce8

case reflect.Slice, reflect.Array:
return sd.DecodeArray(t)

gossamer/codec/decode.go

Lines 300 to 301 in 8fefce8

case reflect.Struct:
return sd.DecodeTuple(t)

Decoding Array Pointer:

gossamer/codec/decode.go

Lines 310 to 311 in 8fefce8

case reflect.Ptr:
v = reflect.ValueOf(t).Elem()

Decoding Tuple (not pointer):

gossamer/codec/decode.go

Lines 375 to 376 in 8fefce8

default:
v = reflect.ValueOf(t)

@noot
Copy link
Contributor

noot commented Nov 6, 2019

we should probably not remove the [32]byte and common.Hash cases, I believe some common types depend on these cases. also, you would think those two cases are duplicates, since common.Hash is an alias for [32]byte, but unfortunately the type switch doesn't recognize that :/

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.

3 participants