Skip to content

Commit

Permalink
Fixes incorrect checks for errors in several tests (prysmaticlabs#7392)
Browse files Browse the repository at this point in the history 
* fixes TestServer_ListAssignments_Pagination_InputOutOfRange
* fixes TestServer_ListValidatorBalances_PaginationOutOfRange
* fix TestServer_ListAttestations_Genesis
* remove redundant TestServer_GetValidatorParticipation_DoesntExist and TestGetDuties_NextEpoch_CantFindValidatorIdx
* Merge branch 'master' into fix-invalid-errcheck-tests
* remove unnecessary import
* fix TestStore_OnAttestation
* fix TestStore_OnAttestationUsingCheckptCache
* fix TestVerifyBlkDescendant
* fix pagination tests
* fix account v2 tests
* fix account v2 tests (remote)
* fix TestServer_JWTInterceptor_BadToken
* Merge refs/heads/master into fix-invalid-errcheck-tests
  • Loading branch information
@farazdagi
farazdagi authored Oct 1, 2020
1 parent 4aea039 commit 95a5b49
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 252 deletions.
113 changes: 46 additions & 67 deletions beacon-chain/blockchain/process_attestation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package blockchain

import (
"context"
"strings"
"testing"

"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -69,47 +68,40 @@ func TestStore_OnAttestation(t *testing.T) {
require.NoError(t, service.beaconDB.SaveState(ctx, s, BlkWithValidStateRoot))

tests := []struct {
name string
a *ethpb.Attestation
wantErr bool
wantErrString string
name string
a *ethpb.Attestation
wantedErr string
}{
{
name: "attestation's data slot not aligned with target vote",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Root: make([]byte, 32)}}},
wantErr: true,
wantErrString: "data slot is not in the same epoch as target 1 != 0",
name: "attestation's data slot not aligned with target vote",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Root: make([]byte, 32)}}},
wantedErr: "data slot is not in the same epoch as target 1 != 0",
},
{
name: "attestation's target root not in db",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: bytesutil.PadTo([]byte{'A'}, 32)}}},
wantErr: true,
wantErrString: "target root does not exist in db",
name: "attestation's target root not in db",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: bytesutil.PadTo([]byte{'A'}, 32)}}},
wantedErr: "target root does not exist in db",
},
{
name: "no pre state for attestations's target block",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: BlkWithOutStateRoot[:]}}},
wantErr: true,
wantErrString: "could not get pre state for epoch 0",
name: "no pre state for attestations's target block",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: BlkWithOutStateRoot[:]}}},
wantedErr: "could not get pre state for epoch 0",
},
{
name: "process attestation doesn't match current epoch",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 100 * params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Epoch: 100,
Root: BlkWithStateBadAttRoot[:]}}},
wantErr: true,
wantErrString: "target epoch 100 does not match current epoch",
wantedErr: "target epoch 100 does not match current epoch",
},
{
name: "process nil attestation",
a: nil,
wantErr: true,
wantErrString: "nil attestation",
name: "process nil attestation",
a: nil,
wantedErr: "nil attestation",
},
{
name: "process nil field (a.Data) in attestation",
a: &ethpb.Attestation{},
wantErr: true,
wantErrString: "nil attestation.Data field",
name: "process nil field (a.Data) in attestation",
a: &ethpb.Attestation{},
wantedErr: "nil attestation.Data field",
},
{
name: "process nil field (a.Target) in attestation",
Expand All @@ -122,20 +114,17 @@ func TestStore_OnAttestation(t *testing.T) {
AggregationBits: make([]byte, 1),
Signature: make([]byte, 96),
},
wantErr: true,
wantErrString: "nil attestation.Data.Target field",
wantedErr: "nil attestation.Data.Target field",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := service.onAttestation(ctx, tt.a)
if tt.wantErr {
if err == nil || !strings.Contains(err.Error(), tt.wantErrString) {
t.Errorf("Store.onAttestation() error = %v, wantErr = %v", err, tt.wantErrString)
}
if tt.wantedErr != "" {
assert.ErrorContains(t, tt.wantedErr, err)
} else {
t.Error(err)
assert.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -191,47 +180,40 @@ func TestStore_OnAttestationUsingCheckptCache(t *testing.T) {
require.NoError(t, service.beaconDB.SaveState(ctx, s, BlkWithValidStateRoot))

tests := []struct {
name string
a *ethpb.Attestation
wantErr bool
wantErrString string
name string
a *ethpb.Attestation
wantedErr string
}{
{
name: "attestation's data slot not aligned with target vote",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Root: make([]byte, 32)}}},
wantErr: true,
wantErrString: "data slot is not in the same epoch as target 1 != 0",
name: "attestation's data slot not aligned with target vote",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Root: make([]byte, 32)}}},
wantedErr: "data slot is not in the same epoch as target 1 != 0",
},
{
name: "attestation's target root not in db",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: bytesutil.PadTo([]byte{'A'}, 32)}}},
wantErr: true,
wantErrString: "target root does not exist in db",
name: "attestation's target root not in db",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: bytesutil.PadTo([]byte{'A'}, 32)}}},
wantedErr: "target root does not exist in db",
},
{
name: "no pre state for attestations's target block",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: BlkWithOutStateRoot[:]}}},
wantErr: true,
wantErrString: "could not get pre state for epoch 0",
name: "no pre state for attestations's target block",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Target: &ethpb.Checkpoint{Root: BlkWithOutStateRoot[:]}}},
wantedErr: "could not get pre state for epoch 0",
},
{
name: "process attestation doesn't match current epoch",
a: &ethpb.Attestation{Data: &ethpb.AttestationData{Slot: 100 * params.BeaconConfig().SlotsPerEpoch, Target: &ethpb.Checkpoint{Epoch: 100,
Root: BlkWithStateBadAttRoot[:]}}},
wantErr: true,
wantErrString: "target epoch 100 does not match current epoch",
wantedErr: "target epoch 100 does not match current epoch",
},
{
name: "process nil attestation",
a: nil,
wantErr: true,
wantErrString: "nil attestation",
name: "process nil attestation",
a: nil,
wantedErr: "nil attestation",
},
{
name: "process nil field (a.Data) in attestation",
a: &ethpb.Attestation{},
wantErr: true,
wantErrString: "nil attestation.Data field",
name: "process nil field (a.Data) in attestation",
a: &ethpb.Attestation{},
wantedErr: "nil attestation.Data field",
},
{
name: "process nil field (a.Target) in attestation",
Expand All @@ -244,20 +226,17 @@ func TestStore_OnAttestationUsingCheckptCache(t *testing.T) {
AggregationBits: make([]byte, 1),
Signature: make([]byte, 96),
},
wantErr: true,
wantErrString: "nil attestation.Data.Target field",
wantedErr: "nil attestation.Data.Target field",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := service.onAttestation(ctx, tt.a)
if tt.wantErr {
if err == nil || !strings.Contains(err.Error(), tt.wantErrString) {
t.Errorf("Store.onAttestation() error = %v, wantErr = %v", err, tt.wantErrString)
}
if tt.wantedErr != "" {
assert.ErrorContains(t, tt.wantedErr, err)
} else {
t.Error(err)
assert.NoError(t, err)
}
})
}
Expand Down
32 changes: 12 additions & 20 deletions beacon-chain/blockchain/process_block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package blockchain

import (
"context"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -675,59 +674,52 @@ func TestVerifyBlkDescendant(t *testing.T) {
finalizedSlot uint64
}
tests := []struct {
name string
args args
shouldError bool
err string
name string
args args
wantedErr string
}{
{
name: "could not get finalized block in block service cache",
args: args{
finalizedRoot: [32]byte{'a'},
},
shouldError: true,
err: "nil finalized block",
wantedErr: "nil finalized block",
},
{
name: "could not get finalized block root in DB",
args: args{
finalizedRoot: r,
parentRoot: [32]byte{'a'},
},
shouldError: true,
err: "could not get finalized block root",
wantedErr: "could not get finalized block root",
},
{
name: "is not descendant",
args: args{
finalizedRoot: r1,
parentRoot: r,
},
shouldError: true,
err: "is not a descendent of the current finalized block slot",
wantedErr: "is not a descendent of the current finalized block slot",
},
{
name: "is descendant",
args: args{
finalizedRoot: r,
parentRoot: r,
},
shouldError: false,
},
}
for _, test := range tests {
for _, tt := range tests {
service, err := NewService(ctx, &Config{BeaconDB: db, StateGen: stategen.New(db, sc), ForkChoiceStore: protoarray.New(0, 0, [32]byte{})})
require.NoError(t, err)
service.finalizedCheckpt = &ethpb.Checkpoint{
Root: test.args.finalizedRoot[:],
Root: tt.args.finalizedRoot[:],
}
err = service.VerifyBlkDescendant(ctx, test.args.parentRoot)
if test.shouldError {
if err == nil || !strings.Contains(err.Error(), test.err) {
t.Error("Did not get wanted error")
}
err = service.VerifyBlkDescendant(ctx, tt.args.parentRoot)
if tt.wantedErr != "" {
assert.ErrorContains(t, tt.wantedErr, err)
} else if err != nil {
t.Error(err)
assert.NoError(t, err)
}
}
}
Expand Down
57 changes: 37 additions & 20 deletions beacon-chain/rpc/beacon/assignments_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/binary"
"fmt"
"strconv"
"strings"
"testing"

"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -80,35 +79,53 @@ func TestServer_ListAssignments_NoResults(t *testing.T) {
}

func TestServer_ListAssignments_Pagination_InputOutOfRange(t *testing.T) {

helpers.ClearCache()
db, sc := dbTest.SetupDB(t)
ctx := context.Background()
setupValidators(t, db, 1)
headState, err := db.HeadState(ctx)
require.NoError(t, err)
count := 100
validators := make([]*ethpb.Validator, 0, count)
for i := 0; i < count; i++ {
pubKey := make([]byte, params.BeaconConfig().BLSPubkeyLength)
withdrawalCred := make([]byte, 32)
binary.LittleEndian.PutUint64(pubKey, uint64(i))
validators = append(validators, &ethpb.Validator{
PublicKey: pubKey,
WithdrawalCredentials: withdrawalCred,
ExitEpoch: params.BeaconConfig().FarFutureEpoch,
EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance,
ActivationEpoch: 0,
})
}

b := testutil.NewBeaconBlock()
require.NoError(t, db.SaveBlock(ctx, b))
gRoot, err := b.Block.HashTreeRoot()
blk := testutil.NewBeaconBlock().Block
blockRoot, err := blk.HashTreeRoot()
require.NoError(t, err)
require.NoError(t, db.SaveGenesisBlockRoot(ctx, gRoot))
require.NoError(t, db.SaveState(ctx, headState, gRoot))

s := testutil.NewBeaconState()
require.NoError(t, s.SetValidators(validators))
require.NoError(t, db.SaveState(ctx, s, blockRoot))
require.NoError(t, db.SaveGenesisBlockRoot(ctx, blockRoot))

bs := &Server{
BeaconDB: db,
BeaconDB: db,
HeadFetcher: &mock.ChainService{
State: s,
},
FinalizationFetcher: &mock.ChainService{
FinalizedCheckPoint: &ethpb.Checkpoint{
Epoch: 0,
},
},
GenesisTimeFetcher: &mock.ChainService{},
StateGen: stategen.New(db, sc),
}

wanted := fmt.Sprintf("page start %d >= list %d", 0, 0)
if _, err := bs.ListValidatorAssignments(
context.Background(),
&ethpb.ListValidatorAssignmentsRequest{
QueryFilter: &ethpb.ListValidatorAssignmentsRequest_Genesis{Genesis: true},
},
); err != nil && !strings.Contains(err.Error(), wanted) {
t.Errorf("Expected error %v, received %v", wanted, err)
}
wanted := fmt.Sprintf("page start %d >= list %d", 500, count)
_, err = bs.ListValidatorAssignments(context.Background(), &ethpb.ListValidatorAssignmentsRequest{
PageToken: strconv.Itoa(2),
QueryFilter: &ethpb.ListValidatorAssignmentsRequest_Genesis{Genesis: true},
})
assert.ErrorContains(t, wanted, err)
}

func TestServer_ListAssignments_Pagination_ExceedsMaxPageSize(t *testing.T) {
Expand Down
20 changes: 0 additions & 20 deletions beacon-chain/rpc/beacon/attestations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"sort"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -77,14 +76,6 @@ func TestServer_ListAttestations_Genesis(t *testing.T) {
},
}

// Should throw an error if no genesis data is found.
if _, err := bs.ListAttestations(ctx, &ethpb.ListAttestationsRequest{
QueryFilter: &ethpb.ListAttestationsRequest_GenesisEpoch{
GenesisEpoch: true,
},
}); err != nil && !strings.Contains(err.Error(), "Could not find genesis") {
t.Fatal(err)
}
att := &ethpb.Attestation{
AggregationBits: bitfield.NewBitlist(0),
Signature: make([]byte, 96),
Expand Down Expand Up @@ -118,17 +109,6 @@ func TestServer_ListAttestations_Genesis(t *testing.T) {
})
require.NoError(t, err)
require.DeepEqual(t, wanted, res)

// Should throw an error if there is more than 1 block
// for the genesis slot.
require.NoError(t, db.SaveBlock(ctx, signedBlock))
if _, err := bs.ListAttestations(ctx, &ethpb.ListAttestationsRequest{
QueryFilter: &ethpb.ListAttestationsRequest_GenesisEpoch{
GenesisEpoch: true,
},
}); err != nil && !strings.Contains(err.Error(), "Found more than 1") {
t.Fatal(err)
}
}

func TestServer_ListAttestations_NoPagination(t *testing.T) {
Expand Down
Loading

0 comments on commit 95a5b49

Please sign in to comment.