From 716401bea1c64a595def1c26db2c084e2442b66e Mon Sep 17 00:00:00 2001
From: gauron99 <fridrich.david19@gmail.com>
Date: Mon, 19 Feb 2024 14:40:13 +0100
Subject: [PATCH] comments

Signed-off-by: gauron99 <fridrich.david19@gmail.com>
---
 cmd/delete_test.go           | 59 +++++++++++++++++++++++++-----------
 cmd/deploy_test.go           |  5 +++
 pkg/functions/client_test.go |  8 ++---
 3 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/cmd/delete_test.go b/cmd/delete_test.go
index 5de13131d0..50f3ad8233 100644
--- a/cmd/delete_test.go
+++ b/cmd/delete_test.go
@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"os"
 	"testing"
 
@@ -108,15 +109,13 @@ func TestDelete_ByName(t *testing.T) {
 	}
 }
 
-// TestDelete_Namespace ensures that the deployed Function is deleted correctly
-// with namespace option
+// TestDelete_Namespace ensures that remover is envoked when --namespace flag is
+// given --> func delete myfunc --namespace myns
 func TestDelete_Namespace(t *testing.T) {
 	var (
-		root      = fromTempDirectory(t)
 		namespace = "myns"
 		remover   = mock.NewRemover()
 		testname  = "testname"
-		err       error
 	)
 
 	remover.RemoveFn = func(_, ns string) error {
@@ -126,27 +125,53 @@ func TestDelete_Namespace(t *testing.T) {
 		return nil
 	}
 
-	// Ensure the extant function's namespace is used
-	f := fn.Function{
-		Name:     testname,
-		Root:     root,
-		Runtime:  "go",
-		Registry: TestRegistry,
-		Deploy: fn.DeploySpec{
-			Namespace: namespace,
-		},
+	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
+	cmd.SetArgs([]string{testname, "--namespace", namespace})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
 	}
 
-	if f, err = fn.New().Init(f); err != nil {
-		t.Fatal(err)
+	if !remover.RemoveInvoked {
+		t.Fatal("remover was not invoked")
 	}
+}
 
-	if err = f.Write(); err != nil {
+// TestDelete_NamespaceFlagPriority ensures that even thought there is
+// a deployed function the namespace flag takes precedence and essentially
+// ignores the the function on disk
+func TestDelete_NamespaceFlagPriority(t *testing.T) {
+	var (
+		root       = fromTempDirectory(t)
+		namespace  = "myns"
+		namespace2 = "myns2"
+		remover    = mock.NewRemover()
+		testname   = "testname"
+		err        error
+	)
+
+	remover.RemoveFn = func(_, ns string) error {
+		if ns != namespace2 {
+			t.Fatalf("expected delete namespace '%v', got '%v'", namespace2, ns)
+		}
+		return nil
+	}
+
+	// Ensure the extant function's namespace is used
+	f := fn.Function{
+		Name:      testname,
+		Root:      root,
+		Runtime:   "go",
+		Registry:  TestRegistry,
+		Namespace: namespace,
+	}
+	client := fn.New()
+	_, _, err = client.New(context.Background(), f)
+	if err != nil {
 		t.Fatal(err)
 	}
 
 	cmd := NewDeleteCmd(NewTestClient(fn.WithRemover(remover)))
-	cmd.SetArgs([]string{testname, "--namespace", namespace})
+	cmd.SetArgs([]string{testname, "--namespace", namespace2})
 	if err := cmd.Execute(); err != nil {
 		t.Fatal(err)
 	}
diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go
index 9787ef39b9..296a2b4cf0 100644
--- a/cmd/deploy_test.go
+++ b/cmd/deploy_test.go
@@ -837,8 +837,13 @@ func TestDeploy_ImageWithDigestErrors(t *testing.T) {
 	}
 }
 
+// TestDeploy_ImageWithDigestDoesntPopulateBuild ensures that when --image is
+// given with digest f.Build.Image is not populated because no image building
+// should happen; f.Deploy.Image should be populated because the image should
+// just be deployed as is (since it already has digest)
 func TestDeploy_ImageWithDigestDoesntPopulateBuild(t *testing.T) {
 	root := fromTempDirectory(t)
+	// image with digest (well almost, atleast in length and syntax)
 	const img = "docker.io/4141gauron3268@sha256:XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
 	// Create a new Function in the temp directory
 	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
diff --git a/pkg/functions/client_test.go b/pkg/functions/client_test.go
index f47b25e7ae..31554a4e50 100644
--- a/pkg/functions/client_test.go
+++ b/pkg/functions/client_test.go
@@ -963,7 +963,7 @@ func TestClient_Deploy_RegistryUpdate(t *testing.T) {
 	}
 }
 
-// TestClient_Deploy_NamespaceUpdate ensures that namespace deploymen has
+// TestClient_Deploy_NamespaceUpdate ensures that namespace deployment has
 // the correct priorities, that means:
 // 'default' gets overridden by 'already deployed' if aplicable and all gets
 // overridden by 'specifically desired namespace'.
@@ -2051,12 +2051,12 @@ func TestClient_BuildCleanFingerprint(t *testing.T) {
 	}
 }
 
-// Test_RemoveInvokedOnOldFunction checks that Remover was invoked after a
-// subsequent redeploy to a new namespace.
+// TestClient_DeployRemoves ensures that the Remover is invoked when a
+// function is moved to a new namespace.
 // specifically: deploy to 'nsone' -> simulate change of namespace with change to
 // f.Namespace -> redeploy to that namespace and expect the remover to be invoked
 // for old Function in ns 'nsone'.
-func TestClient_RemoveInvokedOnOldFunction(t *testing.T) {
+func TestClient_DeployRemoves(t *testing.T) {
 	// Create a temporary directory
 	root, cleanup := Mktemp(t)
 	defer cleanup()