filecoin-project / on-chain-voting

A project repo for Filecoin on-chain voting infrastructure.
2 stars 0 forks source link

Least Authority Initial Audit Suggestion 4 #14

Closed ianconsolata closed 5 months ago

ianconsolata commented 6 months ago

Suggestion 4: Assert Deeper on Tests

Location

Huang-Zexiang commented 5 months ago

@ianconsolata done.

ianconsolata commented 5 months ago

When the issue mentions asserting more deeply, I think they meant use assert.DeepEqual (https://pkg.go.dev/gotest.tools/assert#DeepEqual) where appropriate. I don't see any DeepEqual uses here in those PRs (though you did add many more test cases, thanks!). Are there any situations where DeepQual should be used in the tests?

mor9x00 commented 5 months ago

The assert.Equal (https://github.com/stretchr/testify) function used in these PRs can determine whether two targets have equal types and values, which aligns with the functionality of assert.DeepEqual you mentioned (https://pkg.go.dev/gotest.tools/assert#DeepEqual). Additionally, the github.com/stretchr/testify/assert library is more powerful, actively maintained, and is also the testing library used by Lotus. @ianconsolata

ianconsolata commented 5 months ago

Ah, I see! If you feel like the tests thoroughly cover testing deep equality then I'll trust your judgement.