Skip to content

Commit

Permalink
test(cosmovisor): improve TestParseUpgradeInfoFile with substring ass…
Browse files Browse the repository at this point in the history
…ertions

In trying to investigate the flake reported in #18073, this change
adds the context of the failing test's filepath. This change allows
us to tighten the tests by using assertions on the error instead of
just checking that it is non-nil.
Also one of the tests that claimed it was empty actually had a
space/newline which meant that it wasn't testing the right expectations.

Updates #18073
  • Loading branch information
odeke-em committed Oct 26, 2023
1 parent a755fef commit 8537b5a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 16 deletions.
4 changes: 2 additions & 2 deletions tools/cosmovisor/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func parseUpgradeInfoFile(filename string, disableRecase bool) (upgradetypes.Pla
}

if len(f) == 0 {
return upgradetypes.Plan{}, errors.New("empty upgrade-info.json")
return upgradetypes.Plan{}, fmt.Errorf("empty upgrade-info.json in %q", filename)
}

var upgradePlan upgradetypes.Plan
Expand All @@ -207,5 +207,5 @@ func parseUpgradeInfoFile(filename string, disableRecase bool) (upgradetypes.Pla
upgradePlan.Name = strings.ToLower(upgradePlan.Name)
}

return upgradePlan, err
return upgradePlan, nil
}
24 changes: 11 additions & 13 deletions tools/cosmovisor/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,67 +14,64 @@ func TestParseUpgradeInfoFile(t *testing.T) {
filename string
expectUpgrade upgradetypes.Plan
disableRecase bool
expectErr bool
expectErr string
}{
{
filename: "f1-good.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{Name: "upgrade1", Info: "some info", Height: 123},
expectErr: false,
},
{
filename: "f2-normalized-name.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{Name: "upgrade2", Info: "some info", Height: 125},
expectErr: false,
},
{
filename: "f2-normalized-name.json",
disableRecase: true,
expectUpgrade: upgradetypes.Plan{Name: "Upgrade2", Info: "some info", Height: 125},
expectErr: false,
},
{
filename: "f2-bad-type.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "cannot unmarshal number into Go struct",
},
{
filename: "f2-bad-type-2.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "height must be greater than 0: invalid request",
},
{
filename: "f3-empty.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "empty upgrade-info.json in",
},
{
filename: "f4-empty-obj.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "invalid upgrade-info.json content: name cannot be empty",
},
{
filename: "f5-partial-obj-1.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "height must be greater than 0",
},
{
filename: "f5-partial-obj-2.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "name cannot be empty: invalid request",
},
{
filename: "unknown.json",
filename: "non-existent.json",
disableRecase: false,
expectUpgrade: upgradetypes.Plan{},
expectErr: true,
expectErr: "no such file or directory",
},
}

Expand All @@ -83,8 +80,9 @@ func TestParseUpgradeInfoFile(t *testing.T) {
t.Run(tc.filename, func(t *testing.T) {
require := require.New(t)
ui, err := parseUpgradeInfoFile(filepath.Join(".", "testdata", "upgrade-files", tc.filename), tc.disableRecase)
if tc.expectErr {
if tc.expectErr != "" {
require.Error(err)
require.Contains(err.Error(), tc.expectErr)
} else {
require.NoError(err)
require.Equal(tc.expectUpgrade, ui)
Expand Down
1 change: 0 additions & 1 deletion tools/cosmovisor/testdata/upgrade-files/f3-empty.json
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@

0 comments on commit 8537b5a

Please sign in to comment.