Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

Commit

Permalink
p2p/simulations: fix unmarshal of simulations.Node
Browse files Browse the repository at this point in the history
Making Node.Up field private in 13292ee
broke TestHTTPNetwork and TestHTTPSnapshot. Because the default
UnmarshalJSON does not handle unexported fields.

Important: The fix is partial and not proper to my taste. But I cut
scope as I think the fix may require a change to the current
serialization format. New ticket:
#1177
  • Loading branch information
Ferenc Szabo committed Jan 31, 2019
1 parent 13292ee commit e7fdfbf
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 0 deletions.
19 changes: 19 additions & 0 deletions p2p/simulations/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,25 @@ func (n *Node) MarshalJSON() ([]byte, error) {
})
}

// UnmarshalJSON implements json.Unmarshaler interface so that we don't lose
// Node.up status. IMPORTANT: The implementation is incomplete; we lose
// p2p.NodeInfo.
func (n *Node) UnmarshalJSON(raw []byte) error {
// TODO: How should we turn back NodeInfo into n.Node?
// Ticket: https://github.com/ethersphere/go-ethereum/issues/1177
no := struct {
Config *adapters.NodeConfig `json:"config,omitempty"`
Up bool `json:"up"`
}{}
if err := json.Unmarshal(raw, &no); err != nil {
return err
}

n.SetUp(no.Up)
n.Config = no.Config
return nil
}

// Conn represents a connection between two nodes in the network
type Conn struct {
// One is the node which initiated the connection
Expand Down
98 changes: 98 additions & 0 deletions p2p/simulations/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"encoding/json"
"fmt"
"reflect"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -485,3 +486,100 @@ func benchmarkMinimalServiceTmp(b *testing.B) {
}
}
}

func TestNode_UnmarshalJSON(t *testing.T) {
t.Run(
"test unmarshal of Node up field",
func(t *testing.T) {
runNodeUnmarshalJSON(t, casesNodeUnmarshalJSONUpField())
},
)
}

func runNodeUnmarshalJSON(t *testing.T, tests []nodeUnmarshalTestCase) {
t.Helper()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var got Node
if err := got.UnmarshalJSON([]byte(tt.marshaled)); err != nil {
expectErrorMessageToContain(t, err, tt.wantErr)
}
expectNodeEquality(t, got, tt.want)
})
}
}

type nodeUnmarshalTestCase struct {
name string
marshaled string
want Node
wantErr string
}

func expectErrorMessageToContain(t *testing.T, got error, want string) {
t.Helper()
if got == nil && want == "" {
return
}

if got == nil && want != "" {
t.Errorf("error was expected, got: nil, want: %v", want)
return
}

if !strings.Contains(got.Error(), want) {
t.Errorf(
"unexpected error message, got %v, want: %v",
want,
got,
)
}
}

func expectNodeEquality(t *testing.T, got Node, want Node) {
t.Helper()
if !reflect.DeepEqual(got, want) {
t.Errorf("Node.UnmarshalJSON() = %v, want %v", got, want)
}
}

func casesNodeUnmarshalJSONUpField() []nodeUnmarshalTestCase {
return []nodeUnmarshalTestCase{
{
name: "empty json",
marshaled: "{}",
want: Node{
up: false,
},
},
{
name: "a stopped node",
marshaled: "{\"up\": false}",
want: Node{
up: false,
},
},
{
name: "a running node",
marshaled: "{\"up\": true}",
want: Node{
up: true,
},
},
{
name: "invalid JSON value on valid key",
marshaled: "{\"up\": foo}",
wantErr: "invalid character",
},
{
name: "invalid JSON key and value",
marshaled: "{foo: bar}",
wantErr: "invalid character",
},
{
name: "bool value expected but got something else (string)",
marshaled: "{\"up\": \"true\"}",
wantErr: "cannot unmarshal string into Go struct",
},
}
}

0 comments on commit e7fdfbf

Please sign in to comment.