From 579e103112f3f5caa1e07e1af003a81e20416053 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Wed, 3 Jan 2024 12:23:55 -0500 Subject: [PATCH 01/10] fix load context bug --- cmd/arcaflow/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index b87ca800..332ad2a2 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -235,8 +235,8 @@ func loadContext(dir string) (map[string][]byte, error) { if err != nil { return err } - - if !i.IsDir() { + // only attempt to read regular files + if i.Mode().IsRegular() { fileData, err := os.ReadFile(path) //nolint:gosec if err != nil { return fmt.Errorf("failed to read file from context directory: %s (%w)", path, err) From a579f80a9aa4822a47fc32afe74633143c0b260d Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Wed, 3 Jan 2024 16:18:44 -0500 Subject: [PATCH 02/10] remove dead code add readable test refactor to load context test changes fix linting add test explanation refactor to limit context files read change context establishment --- cmd/arcaflow/main.go | 69 +++++++++++++++++++++------------------ loadfile/loadfile.go | 27 +++++++++++++++ loadfile/loadfile_test.go | 61 ++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 31 deletions(-) create mode 100644 loadfile/loadfile.go create mode 100644 loadfile/loadfile_test.go diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index 332ad2a2..77531a2a 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -5,12 +5,11 @@ import ( "context" "flag" "fmt" + "go.arcalot.io/log/v2" + "go.flow.arcalot.io/engine/loadfile" "os" "os/signal" "path/filepath" - "strings" - - "go.arcalot.io/log/v2" "go.flow.arcalot.io/engine" "go.flow.arcalot.io/engine/config" @@ -38,6 +37,15 @@ const ExitCodeWorkflowErrorOutput = 2 // ExitCodeWorkflowFailed indicates that the workflow execution failed. const ExitCodeWorkflowFailed = 3 +// RequiredFileKeyWorkflow is the key for the workflow file in hash map of files required for execution. +const RequiredFileKeyWorkflow = "workflow" + +// RequiredFileKeyConfig is the key for the config file in hash map of files required for execution. +const RequiredFileKeyConfig = "config" + +// RequiredFileKeyInput is the key for the input file in hash map of files required for execution. +const RequiredFileKeyInput = "input" + func main() { tempLogger := log.New(log.Config{ Level: log.LevelInfo, @@ -117,6 +125,31 @@ Options: } var err error + + absDir, err := filepath.Abs(dir) + if err != nil { + tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %s (%v)", dir, err) + os.Exit(ExitCodeInvalidData) + } + + requiredFiles := map[string]string{ + RequiredFileKeyConfig: configFile, + RequiredFileKeyInput: input, + RequiredFileKeyWorkflow: workflowFile, + } + var requiredFilesAbs = map[string]string{} + for key, f := range requiredFiles { + abspath := f + if !filepath.IsAbs(f) { + abspath = filepath.Join(absDir, f) + } + requiredFilesAbs[key] = abspath + } + var requiredFilesAbsSlice = make([]string, len(requiredFiles)) + for _, f := range requiredFilesAbs { + requiredFilesAbsSlice = append(requiredFilesAbsSlice, f) + } + var configData any = map[any]any{} if configFile != "" { configData, err = loadYamlFile(configFile) @@ -136,7 +169,7 @@ Options: logger := log.New(cfg.Log).WithLabel("source", "main") - dirContext, err := loadContext(dir) + dirContext, err := loadfile.LoadContext(requiredFilesAbsSlice) if err != nil { logger.Errorf("Failed to load configuration file %s (%v)", configFile, err) flag.Usage() @@ -160,7 +193,7 @@ Options: } } - os.Exit(runWorkflow(flow, dirContext, workflowFile, logger, inputData)) + os.Exit(runWorkflow(flow, dirContext, requiredFilesAbs[RequiredFileKeyWorkflow], logger, inputData)) } func runWorkflow(flow engine.WorkflowEngine, dirContext map[string][]byte, workflowFile string, logger log.Logger, inputData []byte) int { @@ -224,32 +257,6 @@ func handleOSInterrupt(ctrlC chan os.Signal, cancel context.CancelFunc, logger l os.Exit(1) } -func loadContext(dir string) (map[string][]byte, error) { - absDir, err := filepath.Abs(dir) - if err != nil { - return nil, fmt.Errorf("failed to obtain absolute path of context directory %s (%w)", dir, err) - } - result := map[string][]byte{} - err = filepath.Walk(absDir, - func(path string, i os.FileInfo, err error) error { - if err != nil { - return err - } - // only attempt to read regular files - if i.Mode().IsRegular() { - fileData, err := os.ReadFile(path) //nolint:gosec - if err != nil { - return fmt.Errorf("failed to read file from context directory: %s (%w)", path, err) - } - path = strings.TrimPrefix(path, absDir) - path = strings.TrimPrefix(path, string([]byte{os.PathSeparator})) - result[path] = fileData - } - return nil - }) - return result, err -} - func loadYamlFile(configFile string) (any, error) { fileContents, err := os.ReadFile(configFile) //nolint:gosec if err != nil { diff --git a/loadfile/loadfile.go b/loadfile/loadfile.go new file mode 100644 index 00000000..5ab93dcd --- /dev/null +++ b/loadfile/loadfile.go @@ -0,0 +1,27 @@ +// Package loadfile provides functions to load files from a directory. +package loadfile + +import ( + "fmt" + "os" + "path/filepath" +) + +// LoadContext reads the contents at each file into a map where the key +// is the absolute filepath and file contents is the value. +func LoadContext(neededFilepaths []string) (map[string][]byte, error) { + result := map[string][]byte{} + var err error + for _, filePath := range neededFilepaths { + absPath, err := filepath.Abs(filePath) + if err != nil { + return nil, fmt.Errorf("failed to obtain absolute path of file %s (%w)", filepath.Base(filePath), err) + } + fileData, err := os.ReadFile(absPath) //nolint:gosec + if err != nil { + return nil, fmt.Errorf("failed to read file from context directory: %s (%w)", absPath, err) + } + result[absPath] = fileData + } + return result, err +} diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go new file mode 100644 index 00000000..7e572457 --- /dev/null +++ b/loadfile/loadfile_test.go @@ -0,0 +1,61 @@ +package loadfile_test + +import ( + "go.arcalot.io/assert" + "go.flow.arcalot.io/engine/loadfile" + "os" + "path/filepath" + "testing" +) + +// This tests the functional behavior of LoadContext when it is given +// a directory of files that cover more than the directory file node +// type 'd' (as seen in *nix systems). Specifically, this test adds +// symbolic link files for a regular file and a directory file. The +// engine should only attempt to read files of a type that the os can +// read (i.e. not throw an error on a call to os.ReadFile()), and +// disregard (not throw an error) files with a type it cannot read. +func TestLoadContext(t *testing.T) { + testdir := "/tmp/loadfile-test" + // cleanup directory even if it's there + _ = os.RemoveAll(testdir) + + assert.NoError(t, os.MkdirAll(testdir, os.ModePerm)) + + // create a directory and a file + dirname := "mydir" + dirpath := filepath.Join(testdir, dirname) + filename := "myfile" + assert.NoError(t, os.MkdirAll(dirpath, os.ModePerm)) + f, err := os.CreateTemp(testdir, filename) + assert.NoError(t, err) + tempfilepath := f.Name() + assert.NoError(t, f.Close()) + + // create symlinks to the above directory and file + symlinkDirname := dirname + "_sym" + symlinkFilepath := tempfilepath + "_sym" + symlinkDirpath := filepath.Join(testdir, symlinkDirname) + assert.NoError(t, os.Symlink(dirpath, symlinkDirpath)) + assert.NoError(t, os.Symlink(tempfilepath, symlinkFilepath)) + + neededFiles := []string{ + tempfilepath, + symlinkFilepath, + } + filemap, err := loadfile.LoadContext(neededFiles) + filemapExp := map[string][]byte{ + tempfilepath: {}, + symlinkFilepath: {}, + } + // assert no error on attempting to read files + // that cannot be read + assert.NoError(t, err) + + // assert only the regular file will be loaded + assert.Equals(t, filemap, filemapExp) + + t.Cleanup(func() { + assert.NoError(t, os.RemoveAll(testdir)) + }) +} From 1aaeccbe2f1da21897eb60136baaa6d0d0be4314 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Thu, 4 Jan 2024 12:10:45 -0500 Subject: [PATCH 03/10] factor out absolute filepath creation --- cmd/arcaflow/main.go | 36 +++++++++++++----------------------- loadfile/loadfile.go | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index 77531a2a..896a0409 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -6,14 +6,12 @@ import ( "flag" "fmt" "go.arcalot.io/log/v2" - "go.flow.arcalot.io/engine/loadfile" - "os" - "os/signal" - "path/filepath" - "go.flow.arcalot.io/engine" "go.flow.arcalot.io/engine/config" + "go.flow.arcalot.io/engine/loadfile" "gopkg.in/yaml.v3" + "os" + "os/signal" ) // These variables are filled using ldflags during the build process with Goreleaser. @@ -126,28 +124,16 @@ Options: var err error - absDir, err := filepath.Abs(dir) - if err != nil { - tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %s (%v)", dir, err) - os.Exit(ExitCodeInvalidData) - } - requiredFiles := map[string]string{ RequiredFileKeyConfig: configFile, RequiredFileKeyInput: input, RequiredFileKeyWorkflow: workflowFile, } - var requiredFilesAbs = map[string]string{} - for key, f := range requiredFiles { - abspath := f - if !filepath.IsAbs(f) { - abspath = filepath.Join(absDir, f) - } - requiredFilesAbs[key] = abspath - } - var requiredFilesAbsSlice = make([]string, len(requiredFiles)) - for _, f := range requiredFilesAbs { - requiredFilesAbsSlice = append(requiredFilesAbsSlice, f) + + requiredFilesAbsPaths, err := loadfile.ContextAbsFilepaths(dir, requiredFiles) + if err != nil { + tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %s (%v)", dir, err) + os.Exit(ExitCodeInvalidData) } var configData any = map[any]any{} @@ -169,6 +155,10 @@ Options: logger := log.New(cfg.Log).WithLabel("source", "main") + var requiredFilesAbsSlice = make([]string, len(requiredFiles)) + for _, f := range requiredFilesAbsPaths { + requiredFilesAbsSlice = append(requiredFilesAbsSlice, f) + } dirContext, err := loadfile.LoadContext(requiredFilesAbsSlice) if err != nil { logger.Errorf("Failed to load configuration file %s (%v)", configFile, err) @@ -193,7 +183,7 @@ Options: } } - os.Exit(runWorkflow(flow, dirContext, requiredFilesAbs[RequiredFileKeyWorkflow], logger, inputData)) + os.Exit(runWorkflow(flow, dirContext, requiredFilesAbsPaths[RequiredFileKeyWorkflow], logger, inputData)) } func runWorkflow(flow engine.WorkflowEngine, dirContext map[string][]byte, workflowFile string, logger log.Logger, inputData []byte) int { diff --git a/loadfile/loadfile.go b/loadfile/loadfile.go index 5ab93dcd..ce4c0912 100644 --- a/loadfile/loadfile.go +++ b/loadfile/loadfile.go @@ -25,3 +25,22 @@ func LoadContext(neededFilepaths []string) (map[string][]byte, error) { } return result, err } + +// ContextAbsFilepaths creates a map of absolute filepaths. If a required +// file is not provided with an absolute path, then it is joined with the +// root directory. +func ContextAbsFilepaths(rootDir string, requiredFiles map[string]string) (map[string]string, error) { + absDir, err := filepath.Abs(rootDir) + if err != nil { + return nil, err + } + requiredFilesAbs := map[string]string{} + for key, f := range requiredFiles { + abspath := f + if !filepath.IsAbs(f) { + abspath = filepath.Join(absDir, f) + } + requiredFilesAbs[key] = abspath + } + return requiredFilesAbs, nil +} From 4e23bfd413d029ff18b7d545fa493b28980b90ad Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Thu, 4 Jan 2024 12:27:18 -0500 Subject: [PATCH 04/10] add error tests rewrite test and add comment fix init slice len fix gosec warning maybe report abs file path config use abs filepath use regular file move code clarify test simplify error msg func comment --- cmd/arcaflow/main.go | 16 +++++---- loadfile/loadfile.go | 17 ++++----- loadfile/loadfile_test.go | 73 ++++++++++++++++++++++++++++++++------- 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index 896a0409..a01cc207 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -130,15 +130,16 @@ Options: RequiredFileKeyWorkflow: workflowFile, } - requiredFilesAbsPaths, err := loadfile.ContextAbsFilepaths(dir, requiredFiles) + requiredFilesAbsPaths, err := loadfile.AbsPathsWithContext(dir, requiredFiles) if err != nil { + flag.Usage() tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %s (%v)", dir, err) os.Exit(ExitCodeInvalidData) } var configData any = map[any]any{} if configFile != "" { - configData, err = loadYamlFile(configFile) + configData, err = loadYamlFile(requiredFilesAbsPaths[RequiredFileKeyConfig]) if err != nil { tempLogger.Errorf("Failed to load configuration file %s (%v)", configFile, err) flag.Usage() @@ -151,17 +152,20 @@ Options: flag.Usage() os.Exit(ExitCodeInvalidData) } - cfg.Log.Stdout = os.Stderr + // now we are ready to instantiate our main logger + cfg.Log.Stdout = os.Stderr logger := log.New(cfg.Log).WithLabel("source", "main") - var requiredFilesAbsSlice = make([]string, len(requiredFiles)) + var requiredFilesAbsSlice = make([]string, len(requiredFilesAbsPaths)) + var j int for _, f := range requiredFilesAbsPaths { - requiredFilesAbsSlice = append(requiredFilesAbsSlice, f) + requiredFilesAbsSlice[j] = f + j++ } dirContext, err := loadfile.LoadContext(requiredFilesAbsSlice) if err != nil { - logger.Errorf("Failed to load configuration file %s (%v)", configFile, err) + logger.Errorf("Failed to load required files into context (%v)", err) flag.Usage() os.Exit(ExitCodeInvalidData) } diff --git a/loadfile/loadfile.go b/loadfile/loadfile.go index ce4c0912..242b5188 100644 --- a/loadfile/loadfile.go +++ b/loadfile/loadfile.go @@ -7,32 +7,33 @@ import ( "path/filepath" ) -// LoadContext reads the contents at each file into a map where the key -// is the absolute filepath and file contents is the value. +// LoadContext reads the content of each file into a map where the key +// is the absolute filepath and the file content is the value. func LoadContext(neededFilepaths []string) (map[string][]byte, error) { result := map[string][]byte{} var err error for _, filePath := range neededFilepaths { absPath, err := filepath.Abs(filePath) if err != nil { - return nil, fmt.Errorf("failed to obtain absolute path of file %s (%w)", filepath.Base(filePath), err) + return nil, fmt.Errorf("error obtaining absolute path of file %s (%w)", + filePath, err) } - fileData, err := os.ReadFile(absPath) //nolint:gosec + fileData, err := os.ReadFile(filepath.Clean(absPath)) if err != nil { - return nil, fmt.Errorf("failed to read file from context directory: %s (%w)", absPath, err) + return nil, fmt.Errorf("error reading file %s (%w)", absPath, err) } result[absPath] = fileData } return result, err } -// ContextAbsFilepaths creates a map of absolute filepaths. If a required +// AbsPathsWithContext creates a map of absolute filepaths. If a required // file is not provided with an absolute path, then it is joined with the // root directory. -func ContextAbsFilepaths(rootDir string, requiredFiles map[string]string) (map[string]string, error) { +func AbsPathsWithContext(rootDir string, requiredFiles map[string]string) (map[string]string, error) { absDir, err := filepath.Abs(rootDir) if err != nil { - return nil, err + return nil, fmt.Errorf("error determining context directory absolute path %s (%w)", rootDir, err) } requiredFilesAbs := map[string]string{} for key, f := range requiredFiles { diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 7e572457..24954564 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -22,39 +22,86 @@ func TestLoadContext(t *testing.T) { assert.NoError(t, os.MkdirAll(testdir, os.ModePerm)) - // create a directory and a file + // create a directory dirname := "mydir" dirpath := filepath.Join(testdir, dirname) - filename := "myfile" assert.NoError(t, os.MkdirAll(dirpath, os.ModePerm)) - f, err := os.CreateTemp(testdir, filename) + + // create a file + filename := "myfile" + filePath := filepath.Join(testdir, filename) + f, err := os.Create(filepath.Clean(filePath)) assert.NoError(t, err) - tempfilepath := f.Name() assert.NoError(t, f.Close()) - // create symlinks to the above directory and file + // create symlink to the directory symlinkDirname := dirname + "_sym" - symlinkFilepath := tempfilepath + "_sym" symlinkDirpath := filepath.Join(testdir, symlinkDirname) assert.NoError(t, os.Symlink(dirpath, symlinkDirpath)) - assert.NoError(t, os.Symlink(tempfilepath, symlinkFilepath)) + + // create symlink to the file + symlinkFilepath := filePath + "_sym" + assert.NoError(t, os.Symlink(filePath, symlinkFilepath)) neededFiles := []string{ - tempfilepath, + filePath, symlinkFilepath, } filemap, err := loadfile.LoadContext(neededFiles) - filemapExp := map[string][]byte{ - tempfilepath: {}, - symlinkFilepath: {}, - } // assert no error on attempting to read files // that cannot be read assert.NoError(t, err) - // assert only the regular file will be loaded + // assert only the regular and symlinked file are loaded + filemapExp := map[string][]byte{ + filePath: {}, + symlinkFilepath: {}, + } assert.Equals(t, filemap, filemapExp) + // error on loading a directory + neededFiles = []string{ + dirpath, + } + _, err = loadfile.LoadContext(neededFiles) + assert.Error(t, err) + + // error on loading a symlink directory + neededFiles = []string{ + symlinkDirpath, + } + _, err = loadfile.LoadContext(neededFiles) + assert.Error(t, err) + + t.Cleanup(func() { + assert.NoError(t, os.RemoveAll(testdir)) + }) +} + +// This tests AbsPathsWithContext joins relative paths with the +// context (root) directory, and passes through absolute paths +// unmodified. +func TestContextAbsFilepaths(t *testing.T) { + testdir, err := os.MkdirTemp(os.TempDir(), "") + assert.NoError(t, err) + + testFilepaths := map[string]string{ + "a": "a.yaml", + "b": "/b.toml", + "c": "../rel/subdir/c.txt", + } + + absPathsExp := map[string]string{ + "a": filepath.Join(testdir, testFilepaths["a"]), + // since the 'b' file has an absolute path, it should be unmodified + "b": "/b.toml", + "c": filepath.Join(testdir, testFilepaths["c"]), + } + + absPathsGot, err := loadfile.AbsPathsWithContext(testdir, testFilepaths) + assert.NoError(t, err) + assert.Equals(t, absPathsExp, absPathsGot) + t.Cleanup(func() { assert.NoError(t, os.RemoveAll(testdir)) }) From 227b53b7207be4ec1128b1912c4d78230deb0bd7 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 13:30:37 -0500 Subject: [PATCH 05/10] assert context files are nil --- loadfile/loadfile_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 24954564..84cd02bf 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -63,15 +63,17 @@ func TestLoadContext(t *testing.T) { neededFiles = []string{ dirpath, } - _, err = loadfile.LoadContext(neededFiles) + ctxFiles, err := loadfile.LoadContext(neededFiles) assert.Error(t, err) + assert.Nil(t, ctxFiles) // error on loading a symlink directory neededFiles = []string{ symlinkDirpath, } - _, err = loadfile.LoadContext(neededFiles) + ctxFiles, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) + assert.Nil(t, ctxFiles) t.Cleanup(func() { assert.NoError(t, os.RemoveAll(testdir)) From cd58b8977b113e7442e38fd41bd1b6586cbfb9de Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 13:47:32 -0500 Subject: [PATCH 06/10] comment test --- loadfile/loadfile_test.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 84cd02bf..1f7f3ebb 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -63,26 +63,26 @@ func TestLoadContext(t *testing.T) { neededFiles = []string{ dirpath, } - ctxFiles, err := loadfile.LoadContext(neededFiles) + _, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) - assert.Nil(t, ctxFiles) + //assert.Nil(t, ctxFiles) // error on loading a symlink directory neededFiles = []string{ symlinkDirpath, } - ctxFiles, err = loadfile.LoadContext(neededFiles) + _, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) - assert.Nil(t, ctxFiles) + //assert.Nil(t, ctxFiles) t.Cleanup(func() { assert.NoError(t, os.RemoveAll(testdir)) }) } -// This tests AbsPathsWithContext joins relative paths with the -// context (root) directory, and passes through absolute paths -// unmodified. +// This tests AbsPathsWithContext which joins relative paths +// with the context (root) directory, and passes through +// absolute paths unmodified. func TestContextAbsFilepaths(t *testing.T) { testdir, err := os.MkdirTemp(os.TempDir(), "") assert.NoError(t, err) @@ -90,13 +90,12 @@ func TestContextAbsFilepaths(t *testing.T) { testFilepaths := map[string]string{ "a": "a.yaml", "b": "/b.toml", - "c": "../rel/subdir/c.txt", + "c": "rel/../subdir/c.txt", } absPathsExp := map[string]string{ "a": filepath.Join(testdir, testFilepaths["a"]), - // since the 'b' file has an absolute path, it should be unmodified - "b": "/b.toml", + "b": testFilepaths["b"], "c": filepath.Join(testdir, testFilepaths["c"]), } From 140473a563fe31b2b63354bbdbe2b94bf55e7e06 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 15:05:05 -0500 Subject: [PATCH 07/10] fix asserts --- loadfile/loadfile_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 1f7f3ebb..20dbe68c 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -63,17 +63,17 @@ func TestLoadContext(t *testing.T) { neededFiles = []string{ dirpath, } - _, err = loadfile.LoadContext(neededFiles) + ctxFiles, err := loadfile.LoadContext(neededFiles) assert.Error(t, err) - //assert.Nil(t, ctxFiles) + assert.Equals(t, ctxFiles, nil) // error on loading a symlink directory neededFiles = []string{ symlinkDirpath, } - _, err = loadfile.LoadContext(neededFiles) + ctxFiles, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) - //assert.Nil(t, ctxFiles) + assert.Equals(t, ctxFiles, nil) t.Cleanup(func() { assert.NoError(t, os.RemoveAll(testdir)) From 242c2466f01c744968726bbbfebabec6a93137eb Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 15:13:34 -0500 Subject: [PATCH 08/10] change log message --- cmd/arcaflow/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/arcaflow/main.go b/cmd/arcaflow/main.go index a01cc207..9be3b58e 100644 --- a/cmd/arcaflow/main.go +++ b/cmd/arcaflow/main.go @@ -133,7 +133,7 @@ Options: requiredFilesAbsPaths, err := loadfile.AbsPathsWithContext(dir, requiredFiles) if err != nil { flag.Usage() - tempLogger.Errorf("Failed to determine absolute path of arcaflow context directory %s (%v)", dir, err) + tempLogger.Errorf("context path resolution failed %s (%v)", dir, err) os.Exit(ExitCodeInvalidData) } From d1a6bd9f2be3ad5d5bb5e51b46f44a9250322ba1 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 16:23:00 -0500 Subject: [PATCH 09/10] test error --- loadfile/loadfile_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 20dbe68c..656a1035 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -63,8 +63,11 @@ func TestLoadContext(t *testing.T) { neededFiles = []string{ dirpath, } + + errFileRead := "reading file" ctxFiles, err := loadfile.LoadContext(neededFiles) assert.Error(t, err) + assert.Contains(t, err.Error(), errFileRead) assert.Equals(t, ctxFiles, nil) // error on loading a symlink directory @@ -73,6 +76,7 @@ func TestLoadContext(t *testing.T) { } ctxFiles, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) + assert.Contains(t, err.Error(), errFileRead) assert.Equals(t, ctxFiles, nil) t.Cleanup(func() { From 65b4e3d32414c4f92f52858d979642f684320204 Mon Sep 17 00:00:00 2001 From: Matthew F Leader Date: Fri, 5 Jan 2024 17:08:24 -0500 Subject: [PATCH 10/10] make test change and upgrade assert --- go.mod | 2 +- go.sum | 7 ++----- loadfile/loadfile_test.go | 4 ++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index fe72af32..0effad3e 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module go.flow.arcalot.io/engine go 1.18 require ( - go.arcalot.io/assert v1.6.0 + go.arcalot.io/assert v1.6.1 go.arcalot.io/dgraph v1.1.0 go.arcalot.io/lang v1.0.0 go.arcalot.io/log/v2 v2.0.0 diff --git a/go.sum b/go.sum index 2147b255..8742ddf4 100644 --- a/go.sum +++ b/go.sum @@ -92,7 +92,6 @@ github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+ github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/imdario/mergo v0.3.6 h1:xTNEAn+kxVO7dTZGu0CegyqKZmoWFI0rF8UxjlB2d28= -github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -152,8 +151,8 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1: github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -go.arcalot.io/assert v1.6.0 h1:iKA8SZZ1MRblMX5QAwwY5RbpR+VNyp//4IU7vo08Xu0= -go.arcalot.io/assert v1.6.0/go.mod h1:Xy3ScX0p9IMY89gdsgexOKxnmDr0nGHG9dV7p8Uxg7w= +go.arcalot.io/assert v1.6.1 h1:pr7wvTE/vZJ4pY0f88R8tXicTOGogkGS1mtY0Qb7GRE= +go.arcalot.io/assert v1.6.1/go.mod h1:Xy3ScX0p9IMY89gdsgexOKxnmDr0nGHG9dV7p8Uxg7w= go.arcalot.io/dgraph v1.1.0 h1:c0LR7+xdUy7Ki6e4nR9rBvK0Upr4Nu49fu+poP/9WMg= go.arcalot.io/dgraph v1.1.0/go.mod h1:FuNv92OgHsJYepD6Unwn+S/4DioBnv06JxQ2BtQct7E= go.arcalot.io/lang v1.0.0 h1:mgDaieT4wWdZTnR4V7+/pgYRmzfU7VZZgIzHccuxAbY= @@ -170,8 +169,6 @@ go.flow.arcalot.io/kubernetesdeployer v0.8.0 h1:UjH/aspPif/k+X65sLWlNDZAW5JlzUfg go.flow.arcalot.io/kubernetesdeployer v0.8.0/go.mod h1:BhERhKpvQMJkrcW9lbBF4kJEe+OGhz2NpSftZIgtVNQ= go.flow.arcalot.io/pluginsdk v0.5.1 h1:ebb2ThAqmjmwGpDyKpd1wEDUisPqPabgARjFohy47Io= go.flow.arcalot.io/pluginsdk v0.5.1/go.mod h1:2s2f//7uOkBjr1QaiWJD/bqDIeLlINJtD1BhiY4aGPM= -go.flow.arcalot.io/podmandeployer v0.6.2 h1:iAAZGgwhxInEVAleakavGruHnW4qsD/v39JpfnTeXiE= -go.flow.arcalot.io/podmandeployer v0.6.2/go.mod h1:BmKbyG2qZG9PMPLkIeXUvKVJfU+AZx+POLvydZN26IY= go.flow.arcalot.io/podmandeployer v0.7.0 h1:bXzWi4IjjLTIftUbH2NPgPiyTb82lzERVgfHP4zpmXI= go.flow.arcalot.io/podmandeployer v0.7.0/go.mod h1:tiWVDNpeNpPrY2GloihwjtnCEzJf8zNxBbiwVGRX7rs= go.flow.arcalot.io/pythondeployer v0.4.0 h1:l8nw6awYMVzgND+ZXdbnNJPYu3V0sgSUFsIzn+SRgh0= diff --git a/loadfile/loadfile_test.go b/loadfile/loadfile_test.go index 656a1035..e5c7dd32 100644 --- a/loadfile/loadfile_test.go +++ b/loadfile/loadfile_test.go @@ -68,7 +68,7 @@ func TestLoadContext(t *testing.T) { ctxFiles, err := loadfile.LoadContext(neededFiles) assert.Error(t, err) assert.Contains(t, err.Error(), errFileRead) - assert.Equals(t, ctxFiles, nil) + assert.Nil(t, ctxFiles) // error on loading a symlink directory neededFiles = []string{ @@ -77,7 +77,7 @@ func TestLoadContext(t *testing.T) { ctxFiles, err = loadfile.LoadContext(neededFiles) assert.Error(t, err) assert.Contains(t, err.Error(), errFileRead) - assert.Equals(t, ctxFiles, nil) + assert.Nil(t, ctxFiles) t.Cleanup(func() { assert.NoError(t, os.RemoveAll(testdir))