refactor/wgcf/optional-keepalive-generation

README.md

@@ -35,8 +35,7 @@ The WireGuard profile will be saved under `wgcf-profile.conf`. For more informat
 
 To send periodic packets that keep NAT mappings alive, use the optional keepalive flag:
 ```bash
-wgcf generate --keepalive       # defaults to 25 seconds
-wgcf generate --keepalive=60    # uses the specified interval
+wgcf generate --keepalive 25    # uses the specified interval
 ```
 This adds `PersistentKeepalive` to the profile's `[Peer]` section. The setting is omitted by default.
 

cmd/generate/generate.go

@@ -29,8 +29,7 @@ var Cmd = &cobra.Command{
 
 func init() {
 	Cmd.PersistentFlags().StringVarP(&profileFile, "profile", "p", "wgcf-profile.conf", "WireGuard profile file")
-	Cmd.PersistentFlags().IntVarP(&keepalive, "keepalive", "k", 0, "Persistent keepalive interval in seconds (default 25)")
-	Cmd.PersistentFlags().Lookup("keepalive").NoOptDefVal = "25"
+	Cmd.PersistentFlags().IntVarP(&keepalive, "keepalive", "k", 0, "Persistent keepalive interval in seconds")
 }
 
 func generateProfile() error {

cmd/generate/generate_test.go

@@ -26,34 +26,58 @@ func TestValidateKeepalive(t *testing.T) {
 }
 
 func TestKeepaliveFlag(t *testing.T) {
-	flags := Cmd.PersistentFlags()
-	flag := flags.Lookup("keepalive")
+	tests := []struct {
+		name    string
+		args    []string
+		want    int
+		wantErr bool
+	}{
+		{name: "omitted", want: 0},
+		{name: "without value", args: []string{"--keepalive"}, wantErr: true},
+		{name: "custom (equal syntax)", args: []string{"--keepalive=60"}, want: 60},
+		{name: "custom (space-separated syntax)", args: []string{"--keepalive", "60"}, want: 60},
+		{name: "short without value", args: []string{"-k"}, wantErr: true},
+		{name: "short custom (equal syntax)", args: []string{"-k=60"}, want: 60},
+		{name: "short custom (space-separated syntax)", args: []string{"-k", "60"}, want: 60},
+		{name: "disabled (equal syntax)", args: []string{"--keepalive=0"}, want: 0},
+		{name: "disabled (space-separated syntax)", args: []string{"--keepalive", "0"}, want: 0},
+		{name: "invalid value (equal syntax)", args: []string{"--keepalive=invalid"}, wantErr: true},
+		{name: "invalid value (space-separated syntax)", args: []string{"--keepalive", "invalid"}, wantErr: true},
+	}
 
-	defer func() {
-		keepalive = 0
-		flag.Changed = false
-	}()
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resetFlag(t, "keepalive")
 
-	keepalive = 0
-	if keepalive != 0 {
-		t.Error()
+			err := Cmd.ParseFlags(test.args)
+			if (err != nil) != test.wantErr {
+				t.Fatalf("ParseFlags(%q) error = %v, wantErr %v", test.args, err, test.wantErr)
+			}
+			if test.wantErr {
+				return
+			}
+			if keepalive != test.want {
+				t.Errorf("keepalive = %d, want %d", keepalive, test.want)
+			}
+		})
 	}
+}
 
-	err := flags.Parse([]string{"--keepalive"})
-	if err != nil {
-		t.Error(err)
+func resetFlag(t *testing.T, name string) {
+	t.Helper()
+	flag := Cmd.PersistentFlags().Lookup(name)
+	if flag == nil {
+		t.Fatalf("flag %q not found", name)
 	}
-	if keepalive != 25 {
-		t.Error()
+	value, changed := flag.Value.String(), flag.Changed
+	t.Cleanup(func() {
+		if err := flag.Value.Set(value); err != nil {
+			t.Errorf("restore flag %q: %v", name, err)
+		}
+		flag.Changed = changed
+	})
+	if err := flag.Value.Set(flag.DefValue); err != nil {
+		t.Fatalf("reset flag %q: %v", name, err)
 	}
-
-	keepalive = 0
 	flag.Changed = false
-	err = flags.Parse([]string{"--keepalive=60"})
-	if err != nil {
-		t.Error(err)
-	}
-	if keepalive != 60 {
-		t.Error()
-	}
 }

wireguard/profile_test.go

@@ -1,7 +1,6 @@
 package wireguard
 
 import (
-	"strings"
 	"testing"
 )
 
@@ -62,22 +61,3 @@ PersistentKeepalive = 25
 		t.Errorf("unexpected profile:\n%s", result)
 	}
 }
-
-func TestGenerateProfileWithCustomKeepalive(t *testing.T) {
-	result, err := generateProfile(&ProfileData{
-		PrivateKey: "1",
-		Address1:   "2",
-		Address2:   "3",
-		PublicKey:  "4",
-		Endpoint:   "5",
-		Keepalive:  60,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	expected := "PersistentKeepalive = 60\n"
-	if !strings.HasSuffix(result, expected) {
-		t.Errorf("expected profile to end with %q, got:\n%s", expected, result)
-	}
-}