Merge pull request #4 from kpitt/improve-test-isolation

Improve test isolation

.golangci.yml

@@ -24,6 +24,7 @@ linters:
   disable:
   - bodyclose
   - contextcheck
+  - deadcode
   - durationcheck
   - dupl
   - exhaustruct
@@ -42,6 +43,7 @@ linters:
   - gosec
   - gosimple
   - govet
+  - ifshort
   - interfacer
   - ireturn
   - lll
@@ -63,7 +65,7 @@ linters:
   - tparallel
   - typecheck
   - unparam
-  - unused
+  - varcheck
   - varnamelen
   - wastedassign
   - wrapcheck

internal/action/action_test.go

@@ -69,6 +69,9 @@ func TestAction(t *testing.T) {
 func TestNew(t *testing.T) {
 	t.Parallel()
 
+	u := gptest.NewUnitTester(t)
+	defer u.Remove()
+
 	td, err := os.MkdirTemp("", "gopass-")
 	require.NoError(t, err)
 	defer func() {

internal/action/generate.go

@@ -57,7 +57,7 @@ func (s *Action) Generate(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	ctx = WithClip(ctx, c.Bool("clip"))
 	force := c.Bool("force")
-	edit := c.Bool("edit") //nolint:ifshort
+	edit := c.Bool("edit")
 
 	args, kvps := parseArgs(c)
 	name := args.Get(0)
@@ -413,7 +413,7 @@ func (s *Action) CompleteGenerate(c *cli.Context) {
 	if c.Args().Len() < 1 {
 		return
 	}
-	needle := c.Args().Get(0) //nolint:ifshort
+	needle := c.Args().Get(0)
 
 	_, err := s.Store.IsInitialized(ctx) // important to make sure the structs are not nil.
 	if err != nil {

internal/action/rcs_test.go

@@ -34,7 +34,7 @@ func TestGit(t *testing.T) { //nolint:paralleltest
 	}()
 
 	// git init
-	c := gptest.CliCtxWithFlags(ctx, t, map[string]string{"username": "foobar", "useremail": "foo.bar@example.org"})
+	c := gptest.CliCtxWithFlags(ctx, t, map[string]string{"name": "foobar", "email": "foo.bar@example.org"})
 	assert.NoError(t, act.RCSInit(c))
 	buf.Reset()
 

internal/config/config_test.go

@@ -1,13 +1,12 @@
 package config_test
 
 import (
-	"os"
-	"path/filepath"
 	"testing"
 
 	_ "github.com/kpitt/gopass/internal/backend/crypto"
 	_ "github.com/kpitt/gopass/internal/backend/storage"
 	"github.com/kpitt/gopass/internal/config"
+	"github.com/kpitt/gopass/tests/gptest"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -16,7 +15,8 @@ func TestHomedir(t *testing.T) { //nolint:paralleltest
 }
 
 func TestNewConfig(t *testing.T) { //nolint:paralleltest
-	assert.NoError(t, os.Setenv("GOPASS_CONFIG", filepath.Join(os.TempDir(), ".gopass.yml")))
+	u := gptest.NewUnitTester(t)
+	defer u.Remove()
 
 	cfg := config.New()
 	cs := cfg.String()
@@ -35,7 +35,8 @@ func TestNewConfig(t *testing.T) { //nolint:paralleltest
 }
 
 func TestSetConfigValue(t *testing.T) { //nolint:paralleltest
-	assert.NoError(t, os.Setenv("GOPASS_CONFIG", filepath.Join(os.TempDir(), ".gopass.yml")))
+	u := gptest.NewUnitTester(t)
+	defer u.Remove()
 
 	cfg := config.New()
 	assert.NoError(t, cfg.SetConfigValue("autoclip", "true"))

internal/store/leaf/reencrypt.go

@@ -16,8 +16,6 @@ import (
 )
 
 // reencrypt will re-encrypt all entries for the current recipients.
-//
-//nolint:ifshort
 func (s *Store) reencrypt(ctx context.Context) error {
 	entries, err := s.List(ctx, "")
 	if err != nil {

main_test.go

@@ -33,6 +33,9 @@ func TestVersionPrinter(t *testing.T) {
 func TestSetupApp(t *testing.T) {
 	t.Parallel()
 
+	u := gptest.NewUnitTester(t)
+	defer u.Remove()
+
 	ctx := context.Background()
 	_, app := setupApp(ctx, "1.9.0", "2022-08-17")
 	assert.NotNil(t, app)
@@ -159,6 +162,9 @@ func testCommands(t *testing.T, c *cli.Context, commands []*cli.Command, prefix
 func TestInitContext(t *testing.T) {
 	t.Parallel()
 
+	u := gptest.NewUnitTester(t)
+	defer u.Remove()
+
 	ctx := context.Background()
 	cfg := config.New()
 

pkg/pwgen/pwrules/aliases.go

@@ -12,10 +12,15 @@ import (
 	"github.com/kpitt/gopass/pkg/fsutil"
 )
 
-var customAliases = map[string][]string{}
+var (
+	customAliases = map[string][]string{}
+	aliasesLoaded = false
+)
 
 // LookupAliases looks up known aliases for the given domain.
 func LookupAliases(domain string) []string {
+	ensureCustomAliases()
+
 	aliases := make([]string, 0, len(genAliases[domain])+len(customAliases[domain]))
 	aliases = append(aliases, genAliases[domain]...)
 	aliases = append(aliases, customAliases[domain]...)
@@ -26,6 +31,8 @@ func LookupAliases(domain string) []string {
 
 // AllAliases returns all aliases.
 func AllAliases() map[string][]string {
+	ensureCustomAliases()
+
 	all := make(map[string][]string, len(genAliases)+len(customAliases))
 	for k, v := range genAliases {
 		all[k] = append(all[k], v...)
@@ -38,17 +45,25 @@ func AllAliases() map[string][]string {
 	return all
 }
 
-func init() {
+func filename() string {
+	return filepath.Join(appdir.UserConfig(), "domain-aliases.json")
+}
+
+func ensureCustomAliases() {
+	if aliasesLoaded || len(customAliases) > 0 {
+		return
+	}
+
 	if err := loadCustomAliases(); err != nil {
 		debug.Log("failed to load custom aliases: %s", err)
 	}
 }
 
-func filename() string {
-	return filepath.Join(appdir.UserConfig(), "domain-aliases.json")
-}
-
 func loadCustomAliases() error {
+	// If `loadCustomAliases` has been called, then aliases are considered to
+	// be loaded even if the file could not be read.
+	aliasesLoaded = true
+
 	fn := filename()
 
 	if !fsutil.IsFile(fn) {
@@ -99,9 +114,7 @@ func saveCustomAliases() error {
 
 // AddCustomAlias adds a custom alias.
 func AddCustomAlias(domain, alias string) error {
-	if len(customAliases) < 1 {
-		_ = loadCustomAliases()
-	}
+	ensureCustomAliases()
 
 	v := make([]string, 0, 1)
 
@@ -124,9 +137,7 @@ func AddCustomAlias(domain, alias string) error {
 
 // RemoveCustomAlias removes a custom alias.
 func RemoveCustomAlias(domain, alias string) error {
-	if len(customAliases) < 1 {
-		_ = loadCustomAliases()
-	}
+	ensureCustomAliases()
 
 	ev, found := customAliases[domain]
 	if !found {
@@ -150,9 +161,7 @@ func RemoveCustomAlias(domain, alias string) error {
 
 // DeleteCustomAlias removes a whole domain.
 func DeleteCustomAlias(domain string) error {
-	if len(customAliases) < 1 {
-		_ = loadCustomAliases()
-	}
+	ensureCustomAliases()
 
 	delete(customAliases, domain)
 

tests/gptest/utils.go

@@ -25,6 +25,11 @@ func AllPathsToSlash(paths []string) []string {
 }
 
 func setupEnv(em map[string]string) error {
+	// Make sure there are no left-over Git environment variables that could
+	// interfere with the expected test results.
+	_ = os.Unsetenv("GIT_AUTHOR_NAME")
+	_ = os.Unsetenv("GIT_AUTHOR_EMAIL")
+
 	for k, v := range em {
 		if err := os.Setenv(k, v); err != nil {
 			return fmt.Errorf("failed to set env %s to %s: %w", k, v, err)