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

more unit test coverage #1167

Merged
merged 5 commits into from
Dec 18, 2024
Merged

Conversation

jchappelow
Copy link
Member

@jchappelow jchappelow commented Dec 17, 2024

This adds more unit test coverage and fixes a number of minor bugs.

"github.com/stretchr/testify/require"
)

func TestGetRawBlockTx(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I actually did a git mv, but added enough new tests that git decided it wasn't similar enough to link. 🤷

@@ -127,7 +127,7 @@ func inferPrecisionAndScale(s string) (precision, scale uint16) {

intPart := uint16(len(parts[0]))
if len(parts) == 1 {
return intPart, 0
return max(1, intPart), 0 // at least 1 for precision with 0 scale
Copy link
Member Author

@jchappelow jchappelow Dec 17, 2024

Choose a reason for hiding this comment

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

In utils/conv/conv_test.go I discovered that "0" could not be used as a valid Decimal. I don't think that is intended, and this case catches when strings.TrimLeft(parts[0], "0") reduced it to an empty string. @brennanjl lmk if a zero decimal is actually not supposed to be allowed. Based on code in other places, it looks like it should have been.

@jchappelow jchappelow force-pushed the dep-badger-ristretto branch 2 times, most recently from 8919708 to 348f677 Compare December 17, 2024 23:04
Comment on lines -22 to -33
s.SetReadDeadline(time.Now().Add(time.Second))

buf := make([]byte, len(discoverPeersMsg))
nr, err := s.Read(buf)
if err != nil && !errors.Is(err, io.EOF) {
n.log.Warn("failed to read peer discovery request", "error", err)
return
}
if nr == 0 { // they hung up
return
}
if string(buf) != discoverPeersMsg {
Copy link
Member Author

Choose a reason for hiding this comment

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

This protocol was an early text based version. The initiator need not send some magic text message to start it off. The stream is specific to the protocol ID and ready to go.

@jchappelow jchappelow merged commit c37fc0c into kwilteam:main Dec 18, 2024
2 checks passed
@jchappelow jchappelow deleted the dep-badger-ristretto branch December 18, 2024 16:12
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