Check errors by identity, not by message text, in tests (closes #49)
check / check (push) Failing after 4s

Tests that asserted a failure by a fragment of its message now use
errors.Is: a refactor returning the wrong error, or wrapping with %v
instead of %w, now fails them. New tests return each exported error of
internal/vault and pkg/bip85 that no test returned, and check wrapped
causes (os.ErrNotExist, ErrMnemonicMismatch through GetSecret,
ErrInvalidPathComponent through DeriveBIP85Entropy). The 999-versions
test moves into package secret to name its unexported error. Checks of
errors no test can name keep their text; they are listed on the issue.

Model: opus-5-5
This commit is contained in:
2026-10-04 20:44:24 +00:00
parent 2adc588ace
commit 79bc021412
19 changed files with 401 additions and 256 deletions
+39 -33
View File
@@ -154,11 +154,10 @@ func newFsFromSnapshot(t *testing.T, tree map[string]string) afero.Fs {
}
// requireRejectedAndUnchanged runs a command on a copy of the state
// directory recorded in before. It requires an error with exactly the
// message of want, so that a later check rejecting the argument does not
// count, and everything under the state directory as it was: the error
// alone proves nothing, since it could come after the vault had already
// been deleted.
// directory recorded in before. It requires the error want, so that a later
// check rejecting the argument does not count, and everything under the
// state directory as it was: the error alone proves nothing, since it could
// come after the vault had already been deleted.
func requireRejectedAndUnchanged(
t *testing.T, before map[string]string, want error,
run func(c *cli.Instance) error,
@@ -170,7 +169,7 @@ func requireRejectedAndUnchanged(
err := run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
require.Equal(t, before, snapshotStateDir(t, fs))
require.EqualError(t, err, want.Error())
require.ErrorIs(t, err, want)
}
// TestInvalidSecretNameLeavesVaultsUnchanged is a regression test for
@@ -193,77 +192,76 @@ func TestInvalidSecretNameLeavesVaultsUnchanged(t *testing.T) {
cmd := &cobra.Command{}
tests := []struct {
command string
rejected string // the secret name the command must reject
run func(c *cli.Instance) error
command string
run func(c *cli.Instance) error
}{
{"rm --force ..", "..", func(c *cli.Instance) error {
{"rm --force ..", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "..", true)
}},
{"rm --force .", ".", func(c *cli.Instance) error {
{"rm --force .", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, ".", true)
}},
{`rm --force ""`, "", func(c *cli.Instance) error {
{`rm --force ""`, func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "", true)
}},
{"rm --force ../../etc", "../../etc", func(c *cli.Instance) error {
{"rm --force ../../etc", func(c *cli.Instance) error {
return c.RemoveSecret(cmd, "../../etc", true)
}},
{"mv --force .. x", "..", func(c *cli.Instance) error {
{"mv --force .. x", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "..", "x", true)
}},
{"mv --force x ..", "..", func(c *cli.Instance) error {
{"mv --force x ..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "x", "..", true)
}},
{`mv --force x ""`, "", func(c *cli.Instance) error {
{`mv --force x ""`, func(c *cli.Instance) error {
return c.MoveSecret(cmd, "x", "", true)
}},
// "work" is not the current vault: a move within it must not
// select it when a name is rejected.
{"mv --force work:.. work:x", "..", func(c *cli.Instance) error {
{"mv --force work:.. work:x", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "work:..", "work:x", true)
}},
{"mv --force work:x work:..", "..", func(c *cli.Instance) error {
{"mv --force work:x work:..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "work:x", "work:..", true)
}},
{"mv --force default:.. work", "..", func(c *cli.Instance) error {
{"mv --force default:.. work", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:..", "work", true)
}},
{"mv --force default:.. work:y", "..", func(c *cli.Instance) error {
{"mv --force default:.. work:y", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:..", "work:y", true)
}},
{"mv --force default:x work:..", "..", func(c *cli.Instance) error {
{"mv --force default:x work:..", func(c *cli.Instance) error {
return c.MoveSecret(cmd, "default:x", "work:..", true)
}},
{"import --force ..", "..", func(c *cli.Instance) error {
{"import --force ..", func(c *cli.Instance) error {
return c.ImportSecret(cmd, "..", missingFile, true)
}},
{"import --force .", ".", func(c *cli.Instance) error {
{"import --force .", func(c *cli.Instance) error {
return c.ImportSecret(cmd, ".", missingFile, true)
}},
{"import --force ../../etc", "../../etc", func(c *cli.Instance) error {
{"import --force ../../etc", func(c *cli.Instance) error {
return c.ImportSecret(cmd, "../../etc", missingFile, true)
}},
{"version list ..", "..", func(c *cli.Instance) error {
{"version list ..", func(c *cli.Instance) error {
return c.ListVersions(cmd, "..")
}},
{"version promote ..", "..", func(c *cli.Instance) error {
{"version promote ..", func(c *cli.Instance) error {
return c.PromoteVersion(cmd, "..", testVersion)
}},
{"version rm --force ..", "..", func(c *cli.Instance) error {
{"version rm --force ..", func(c *cli.Instance) error {
return c.RemoveVersion(cmd, "..", testVersion, true)
}},
{"encrypt ..", "..", func(c *cli.Instance) error {
{"encrypt ..", func(c *cli.Instance) error {
return c.Encrypt("..", "", "")
}},
{"decrypt ..", "..", func(c *cli.Instance) error {
{"decrypt ..", func(c *cli.Instance) error {
return c.Decrypt("..", "", "")
}},
}
for _, tt := range tests {
t.Run(tt.command, func(t *testing.T) {
requireRejectedAndUnchanged(t, before, vault.ValidateSecretName(tt.rejected), tt.run)
requireRejectedAndUnchanged(t, before, vault.ErrInvalidSecretName, tt.run)
})
}
}
@@ -299,10 +297,18 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
for _, tt := range commands {
for _, version := range []string{"", ".", "..", "../../..", "a/b"} {
t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) {
fs := newFsFromSnapshot(t, before)
err := tt.run(cli.NewCLIInstanceWithStateDir(fs, testStateDir), version)
require.Equal(t, before, snapshotStateDir(t, fs))
// Compared as text: `version rm` and `version promote` return
// internal/cli's own error of this text, which errors.Is does
// not match to vault.ErrVersionNotFound.
want := fmt.Errorf("version '%s' %w '%s'",
version, vault.ErrVersionNotFound, "x")
requireRejectedAndUnchanged(t, before, want,
func(c *cli.Instance) error { return tt.run(c, version) })
require.EqualError(t, err, want.Error())
})
}
}
@@ -355,7 +361,7 @@ func TestInvalidVaultNameLeavesStateUnchanged(t *testing.T) {
for _, tt := range commands {
for _, name := range []string{"", ".", "..", "a/b"} {
t.Run(fmt.Sprintf(tt.command, name), func(t *testing.T) {
requireRejectedAndUnchanged(t, before, vault.ValidateVaultName(name),
requireRejectedAndUnchanged(t, before, vault.ErrInvalidVaultName,
func(c *cli.Instance) error {
c.Mnemonic = mnemonic
c.UnlockPassphrase = passphrase