From 347e9d07657f358c297a21aabb6224c38502a08f Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Wed, 24 Apr 2024 21:49:34 +0200
Subject: [PATCH] complete RESITC_HOST environment handling & test

---
 changelog/unreleased/issue-4733 |  3 +-
 cmd/restic/find.go              | 16 ++++-----
 cmd/restic/find_test.go         | 61 +++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 11 deletions(-)
 create mode 100644 cmd/restic/find_test.go

diff --git a/changelog/unreleased/issue-4733 b/changelog/unreleased/issue-4733
index 250c2ba68..1fc271587 100644
--- a/changelog/unreleased/issue-4733
+++ b/changelog/unreleased/issue-4733
@@ -6,4 +6,5 @@ grouoping snapshots. They now permit selecting the hostname via the
 environment variable `RESTIC_HOST`. `--host` still takes precedence over the
 environment variable.
 
-https://github.com/restic/restic/issues/4733
\ No newline at end of file
+https://github.com/restic/restic/issues/4733
+https://github.com/restic/restic/pull/4734
diff --git a/cmd/restic/find.go b/cmd/restic/find.go
index 7c28b3be5..c7754d5d9 100644
--- a/cmd/restic/find.go
+++ b/cmd/restic/find.go
@@ -19,11 +19,9 @@ func initMultiSnapshotFilter(flags *pflag.FlagSet, filt *restic.SnapshotFilter,
 	flags.Var(&filt.Tags, "tag", "only consider snapshots including `tag[,tag,...]` (can be specified multiple times)")
 	flags.StringArrayVar(&filt.Paths, "path", nil, "only consider snapshots including this (absolute) `path` (can be specified multiple times)")
 
-	if len(filt.Hosts) == 0 {
-		// parse host from env, if not exists or empty the default value will be used
-		if host := os.Getenv("RESTIC_HOST"); host != "" {
-			filt.Hosts = []string{host}
-		}
+	// set default based on env if set
+	if host := os.Getenv("RESTIC_HOST"); host != "" {
+		filt.Hosts = []string{host}
 	}
 }
 
@@ -34,11 +32,9 @@ func initSingleSnapshotFilter(flags *pflag.FlagSet, filt *restic.SnapshotFilter)
 	flags.Var(&filt.Tags, "tag", "only consider snapshots including `tag[,tag,...]`, when snapshot ID \"latest\" is given (can be specified multiple times)")
 	flags.StringArrayVar(&filt.Paths, "path", nil, "only consider snapshots including this (absolute) `path`, when snapshot ID \"latest\" is given (can be specified multiple times)")
 
-	if len(filt.Hosts) == 0 {
-		// parse host from env, if not exists or empty the default value will be used
-		if host := os.Getenv("RESTIC_HOST"); host != "" {
-			filt.Hosts = []string{host}
-		}
+	// set default based on env if set
+	if host := os.Getenv("RESTIC_HOST"); host != "" {
+		filt.Hosts = []string{host}
 	}
 }
 
diff --git a/cmd/restic/find_test.go b/cmd/restic/find_test.go
new file mode 100644
index 000000000..a98a14f04
--- /dev/null
+++ b/cmd/restic/find_test.go
@@ -0,0 +1,61 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/restic/restic/internal/restic"
+	rtest "github.com/restic/restic/internal/test"
+	"github.com/spf13/pflag"
+)
+
+func TestSnapshotFilter(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		args     []string
+		expected []string
+		env      string
+	}{
+		{
+			"no value",
+			[]string{},
+			nil,
+			"",
+		},
+		{
+			"args only",
+			[]string{"--host", "abc"},
+			[]string{"abc"},
+			"",
+		},
+		{
+			"env default",
+			[]string{},
+			[]string{"def"},
+			"def",
+		},
+		{
+			"both",
+			[]string{"--host", "abc"},
+			[]string{"abc"},
+			"def",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Setenv("RESTIC_HOST", test.env)
+
+			for _, mode := range []bool{false, true} {
+				set := pflag.NewFlagSet("test", pflag.PanicOnError)
+				flt := &restic.SnapshotFilter{}
+				if mode {
+					initMultiSnapshotFilter(set, flt, false)
+				} else {
+					initSingleSnapshotFilter(set, flt)
+				}
+				err := set.Parse(test.args)
+				rtest.OK(t, err)
+
+				rtest.Equals(t, test.expected, flt.Hosts, "unexpected hosts")
+			}
+		})
+	}
+}