Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test for func run with digested image override #2650

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,79 @@ func TestRun_CorrectImage(t *testing.T) {
})
}
}

// TestRun_DirectOverride tests that an --image passed after a function has
// already been build, the given --image with digest will override built function
func TestRun_DirectOverride(t *testing.T) {
const overrideImage = "registry/myrepo/myimage@sha256:0000000000000000000000000000000000000000000000000000000000000000"
root := FromTempDirectory(t)
runner := mock.NewRunner()

runner.RunFn = func(_ context.Context, f fn.Function, _ time.Duration) (*fn.Job, error) {
if f.Build.Image != overrideImage {
return nil, fmt.Errorf("Expected image to be overridden with '%v' but got: '%v'", overrideImage, f.Build.Image)
}
errs := make(chan error, 1)
stop := func() error { return nil }
return fn.NewJob(f, "127.0.0.1", "8080", errs, stop, false)
}

builder1 := mock.NewBuilder()

// SETUP THE ENVIRONMENT & SITUATION
gauron99 marked this conversation as resolved.
Show resolved Hide resolved
// create function
_, err := fn.New().Init(fn.Function{Root: root, Runtime: "go"})
if err != nil {
t.Fatal(err)
}

// build function
cmdBuild := NewBuildCmd(NewTestClient(fn.WithBuilder(builder1), fn.WithRegistry("example.com/ns-to-override")))
if err := cmdBuild.Execute(); err != nil {
t.Fatal(err)
}

// fetch the functions state
_, err = fn.NewFunction(root)
if err != nil {
t.Fatal(err)
}

// builder for 'func run' -- shall not be invoked
builder2 := mock.NewBuilder()
builder2.BuildFn = func(f fn.Function) error {
return fmt.Errorf("should not be invoked")
}

// RUN THE ACTUAL TESTED COMMAND
cmd := NewRunCmd(NewTestClient(
fn.WithRunner(runner),
fn.WithBuilder(builder2),
fn.WithRegistry("ghcr.com/reg"),
))
cmd.SetArgs([]string{fmt.Sprintf("--image=%s", overrideImage)})

// run function with above argument
ctx, cancel := context.WithCancel(context.Background())
runErrCh := make(chan error, 1)
go func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could get rid of this asynchronicity by closing err channel in the runner:

diff --git a/cmd/run_test.go b/cmd/run_test.go
index 7340ca59f..88130ea83 100644
--- a/cmd/run_test.go
+++ b/cmd/run_test.go
@@ -398,7 +398,8 @@ func TestRun_DirectOverride(t *testing.T) {
 		if f.Build.Image != overrideImage {
 			return nil, fmt.Errorf("Expected image to be overridden with '%v' but got: '%v'", overrideImage, f.Build.Image)
 		}
-		errs := make(chan error, 1)
+		errs := make(chan error)
+		close(errs)
 		stop := func() error { return nil }
 		return fn.NewJob(f, "127.0.0.1", "8080", errs, stop, false)
 	}
@@ -440,25 +441,16 @@ func TestRun_DirectOverride(t *testing.T) {
 
 	// run function with above argument
 	ctx, cancel := context.WithCancel(context.Background())
-	runErrCh := make(chan error, 1)
-	go func() {
-		_, err := cmd.ExecuteContextC(ctx)
+	defer cancel()
+
+	err = cmd.ExecuteContext(ctx)
 	if err != nil {
-			runErrCh <- err // error was not expected
-			return
+		t.Fatal(err)
 	}
 
 	// Ensure invocation doesnt happen for the second time as the image was
 	// provided with a digest (should not build)
 	if builder2.BuildInvoked {
-			runErrCh <- fmt.Errorf("Function was not expected to build again but it did")
-		}
-
-		close(runErrCh) // release the waiting parent process
-	}()
-	cancel() // trigger the return of cmd.ExecuteContextC in the routine
-	<-ctx.Done()
-	if err := <-runErrCh; err != nil { // wait for completion of assertions
-		t.Fatal(err)
+		t.Fatal("Function was not expected to build again but it did")
 	}
 }

_, err := cmd.ExecuteContextC(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably could be just err := cmd.ExecuteContext(ctx)

if err != nil {
runErrCh <- err // error was not expected
return
}

// Ensure invocation doesnt happen for the second time as the image was
gauron99 marked this conversation as resolved.
Show resolved Hide resolved
// provided with a digest (should not build)
if builder2.BuildInvoked {
runErrCh <- fmt.Errorf("Function was not expected to build again but it did")
}

close(runErrCh) // release the waiting parent process
}()
cancel() // trigger the return of cmd.ExecuteContextC in the routine
<-ctx.Done()
if err := <-runErrCh; err != nil { // wait for completion of assertions
t.Fatal(err)
}
}
Loading