fix: function errors prematurely (#1373)

The NewFunction constructor should only error on invalid paths; not
when the given path has not yet been initialized.
This commit is contained in:
Luke Kingland 2022-10-21 21:56:14 +09:00 committed by GitHub
parent 034e2de849
commit fe81e235d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 69 additions and 12 deletions

View File

@ -173,10 +173,27 @@ func NewFunction(path string) (f Function, err error) {
f.Root = path // path is not persisted, as this is the purview of the FS itself
f.Build.BuilderImages = make(map[string]string)
f.Deploy.Annotations = make(map[string]string)
// Path must exist and be a directory
fd, err := os.Stat(path)
if err != nil {
return f, err
}
if !fd.IsDir() {
return f, fmt.Errorf("function path must be a directory")
}
// If no func.yaml in directory, return the default function which will
// have f.Initialized() == false
var filename = filepath.Join(path, FunctionFile)
if _, err = os.Stat(filename); err != nil {
if os.IsNotExist(err) {
err = nil
}
return
}
// Path is valid and func.yaml exists: load it
bb, err := os.ReadFile(filename)
if err != nil {
return

View File

@ -4,6 +4,8 @@
package function_test
import (
"os"
"path/filepath"
"strings"
"testing"
@ -13,6 +15,33 @@ import (
. "knative.dev/func/testing"
)
// TestFunction_PathErrors ensures that instantiating a function errors if
// the path does not exist or is not a directory, but does not require the
// path contain an initialized function.
func TestFunction_PathErrors(t *testing.T) {
root, rm := Mktemp(t)
defer rm()
_, err := fn.NewFunction(root)
if err != nil {
t.Fatalf("an empty but valid directory path should not error. got '%v'", err)
}
_, err = fn.NewFunction(filepath.Join(root, "nonexistent"))
if err == nil {
t.Fatalf("a nonexistent path should error")
}
if err := os.WriteFile("filepath", []byte{}, os.ModePerm); err != nil {
t.Fatal(err)
}
_, err = fn.NewFunction(filepath.Join(root, "filepath"))
if err == nil {
t.Fatalf("an invalid path (non-directory) should error")
}
}
// TestFunction_WriteIdempotency ensures that a function can be written repeatedly
// without change.
func TestFunction_WriteIdempotency(t *testing.T) {
@ -57,8 +86,11 @@ func TestFunction_NameDefault(t *testing.T) {
root := "testdata/testFunctionNameDefault"
defer Using(t, root)()
f, err := fn.NewFunction(root)
if err == nil {
t.Fatal("expected error instantiating a nonexistent function")
if err != nil {
t.Fatal(err)
}
if f.Initialized() {
t.Fatal("a function about an empty, but valid path, shold not be initialized")
}
// Create the function at the path

View File

@ -5,6 +5,7 @@ package function
import (
"context"
"fmt"
"runtime"
"strings"
"testing"
@ -69,15 +70,15 @@ func TestInstance_RemoteErrors(t *testing.T) {
if err := New().Create(Function{Runtime: "go", Root: root}); err != nil {
t.Fatal(err)
}
// Load the function
_, err := NewFunction(root)
if err != nil {
if _, err := NewFunction(root); err != nil {
t.Fatal(err)
}
var badRoot = "func.yaml: no such file or directory"
var badRoot = "no such file or directory"
if runtime.GOOS == "windows" {
badRoot = "The system cannot find the path specified"
badRoot = "The system cannot find the file specified"
}
tests := []struct {
@ -86,7 +87,7 @@ func TestInstance_RemoteErrors(t *testing.T) {
want string
}{
{
name: "foo",
name: "",
root: "foo", // bad root
want: badRoot,
},
@ -96,11 +97,18 @@ func TestInstance_RemoteErrors(t *testing.T) {
want: "name passed does not match name of the function at root",
},
}
for _, tt := range tests {
i := Instances{}
if _, err := i.Remote(context.TODO(), tt.name, tt.root); !strings.Contains(err.Error(), tt.want) {
t.Errorf("Remote() %v error = %v, wantErr %v", "Mismatched name and root", err.Error(), tt.want)
}
for _, test := range tests {
testName := fmt.Sprintf("name '%v' and root '%v'", test.name, test.root)
t.Run(testName, func(t *testing.T) {
i := Instances{}
_, err := i.Remote(context.Background(), test.name, test.root)
if err == nil {
t.Fatal("did not receive expected error")
}
if !strings.Contains(err.Error(), test.want) {
t.Errorf("expected error to contain '%v', got '%v'", test.want, err)
}
})
}
}