From 8c4899f03a0fd9d7987f56ad8395f2724be832d7 Mon Sep 17 00:00:00 2001 From: yohamta Date: Mon, 2 May 2022 16:02:26 +0900 Subject: [PATCH] Add tests --- internal/controller/controller.go | 56 ++++++++----------- internal/controller/controller_test.go | 34 +++++++++++ internal/controller/dag.go | 5 -- internal/controller/dag_test.go | 17 ++++++ internal/database/writer.go | 30 +++------- internal/models/node.go | 3 + internal/models/status.go | 16 ++---- internal/utils/utils.go | 18 +----- internal/utils/utils_test.go | 44 ++++++++++++++- tests/testdata/controller_config_error.yaml | 3 + .../controller_update_status_failed.yaml | 4 ++ 11 files changed, 143 insertions(+), 87 deletions(-) create mode 100644 internal/controller/dag_test.go create mode 100644 tests/testdata/controller_config_error.yaml create mode 100644 tests/testdata/controller_update_status_failed.yaml diff --git a/internal/controller/controller.go b/internal/controller/controller.go index f92210a0b..d93c163cb 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -38,22 +38,19 @@ func GetDAGs(dir string) (dags []*DAG, errs []string, err error) { return } fis, err := ioutil.ReadDir(dir) - if err != nil { - log.Printf("%v", err) - } + utils.LogIgnoreErr("read DAGs directory", err) for _, fi := range fis { - if filepath.Ext(fi.Name()) != ".yaml" { - continue - } - dag, err := fromConfig(filepath.Join(dir, fi.Name()), true) - if err != nil { - log.Printf("%v", err) + ex := filepath.Ext(fi.Name()) + if ex == ".yaml" || ex == ".yml" { + dag, err := fromConfig(filepath.Join(dir, fi.Name()), true) + utils.LogIgnoreErr("read DAG config", err) if dag == nil { - errs = append(errs, err.Error()) + errs = append(errs, + fmt.Sprintf("reading %s failed: %s", fi.Name(), err)) continue } + dags = append(dags, dag) } - dags = append(dags, dag) } return dags, errs, nil } @@ -89,9 +86,7 @@ func (c *controller) Start(bin string, workDir string, params string) (err error cmd.Env = os.Environ() defer cmd.Wait() err = cmd.Start() - if err != nil { - log.Printf("failed to start a DAG: %v", err) - } + utils.LogIgnoreErr("starting a DAG", err) }() time.Sleep(time.Millisecond * 500) return @@ -139,20 +134,21 @@ func (s *controller) GetLastStatus() (*models.Status, error) { if err == nil { return models.StatusFromJson(ret) } - if err != nil && errors.Is(err, sock.ErrTimeout) { - return nil, err - } - db := database.New(database.DefaultConfig()) - status, err := db.ReadStatusToday(s.cfg.ConfigPath) - if err != nil { - var readErr error = nil - if err != database.ErrNoStatusDataToday && err != database.ErrNoStatusData { - fmt.Printf("read status failed : %s", err) - readErr = err + utils.LogIgnoreErr("get last status", err) + if err == nil || !errors.Is(err, sock.ErrTimeout) { + db := database.New(database.DefaultConfig()) + status, err := db.ReadStatusToday(s.cfg.ConfigPath) + if err != nil { + var readErr error = nil + if err != database.ErrNoStatusDataToday && err != database.ErrNoStatusData { + fmt.Printf("read status failed : %s", err) + readErr = err + } + return defaultStatus(s.cfg), readErr } - return defaultStatus(s.cfg), readErr + return status, nil } - return status, nil + return nil, err } func (s *controller) GetStatusByRequestId(requestId string) (*models.Status, error) { @@ -177,8 +173,7 @@ func (s *controller) UpdateStatus(status *models.Status) error { if errors.Is(err, sock.ErrTimeout) { return err } - } - if err == nil { + } else { ss, err := models.StatusFromJson(res) if err != nil { return err @@ -200,10 +195,7 @@ func (s *controller) UpdateStatus(status *models.Status) error { if err := w.Write(status); err != nil { return err } - if err := w.Close(); err != nil { - return err - } - return nil + return w.Close() } func defaultStatus(cfg *config.Config) *models.Status { diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 1139823be..4f590658c 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -116,6 +116,40 @@ func TestUpdateStatus(t *testing.T) { require.Equal(t, scheduler.NodeStatus_Error, updated.Nodes[0].Status) } +func TestUpdateStatusError(t *testing.T) { + file := testConfig("controller_update_status_failed.yaml") + + dag, err := controller.FromConfig(file) + require.NoError(t, err) + + a := agent.Agent{Config: &agent.Config{ + DAG: dag.Config, + }} + + go func() { + err = a.Run() + require.NoError(t, err) + }() + + time.Sleep(time.Millisecond * 30) + + c := controller.New(dag.Config) + st, err := c.GetLastStatus() + require.NoError(t, err) + require.Equal(t, scheduler.SchedulerStatus_Running, st.Status) + + st.Nodes[0].Status = scheduler.NodeStatus_Error + err = c.UpdateStatus(st) + require.Error(t, err) + + err = c.Stop() + require.NoError(t, err) + + st.RequestId = "invalid request id" + err = c.UpdateStatus(st) + require.Error(t, err) +} + func TestStartStop(t *testing.T) { file := testConfig("controller_start.yaml") dag, err := controller.FromConfig(file) diff --git a/internal/controller/dag.go b/internal/controller/dag.go index a709508de..6bcd8e1f7 100644 --- a/internal/controller/dag.go +++ b/internal/controller/dag.go @@ -1,13 +1,11 @@ package controller import ( - "fmt" "path/filepath" "github.com/yohamta/dagu/internal/config" "github.com/yohamta/dagu/internal/models" "github.com/yohamta/dagu/internal/scheduler" - "github.com/yohamta/dagu/internal/utils" ) type DAG struct { @@ -20,9 +18,6 @@ type DAG struct { } func FromConfig(file string) (*DAG, error) { - if !utils.FileExists(file) { - return nil, fmt.Errorf("file not found: %s", file) - } return fromConfig(file, false) } diff --git a/internal/controller/dag_test.go b/internal/controller/dag_test.go new file mode 100644 index 000000000..b33f02e59 --- /dev/null +++ b/internal/controller/dag_test.go @@ -0,0 +1,17 @@ +package controller_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/yohamta/dagu/internal/controller" +) + +func TestLoadConfig(t *testing.T) { + file := testConfig("controller_config_error.yaml") + dag, err := controller.FromConfig(file) + require.Error(t, err) + require.NotNil(t, dag) + require.Error(t, dag.Error) + require.Equal(t, file, dag.Config.ConfigPath) +} diff --git a/internal/database/writer.go b/internal/database/writer.go index f1773d0c4..63091c043 100644 --- a/internal/database/writer.go +++ b/internal/database/writer.go @@ -2,7 +2,6 @@ package database import ( "bufio" - "fmt" "os" "path" "strings" @@ -20,43 +19,32 @@ type Writer struct { closed bool } -func (w *Writer) Open() error { - if w.closed { - return fmt.Errorf("file was already closed") - } - var err error +func (w *Writer) Open() (err error) { os.MkdirAll(path.Dir(w.Target), 0755) w.file, err = utils.OpenOrCreateFile(w.Target) - if err != nil { - return err + if err == nil { + w.writer = bufio.NewWriter(w.file) } - w.writer = bufio.NewWriter(w.file) - return nil + return } func (w *Writer) Write(st *models.Status) error { w.mu.Lock() defer w.mu.Unlock() - if w.writer == nil || w.file == nil { - return fmt.Errorf("file was not opened") - } jsonb, _ := st.ToJson() str := strings.ReplaceAll(string(jsonb), "\n", " ") str = strings.ReplaceAll(str, "\r", " ") _, err := w.writer.WriteString(str + "\n") - if err != nil { - return err - } + utils.LogIgnoreErr("write status", err) return w.writer.Flush() } -func (w *Writer) Close() error { +func (w *Writer) Close() (err error) { if !w.closed { - if err := w.writer.Flush(); err != nil { - return err - } + err = w.writer.Flush() + utils.LogIgnoreErr("flush file", err) w.file.Close() w.closed = true } - return nil + return err } diff --git a/internal/models/node.go b/internal/models/node.go index 499efcb2a..bd5e1f871 100644 --- a/internal/models/node.go +++ b/internal/models/node.go @@ -119,6 +119,9 @@ func graphNode(val string) string { } func fromStepWithDefValues(s *config.Step) *Node { + if s == nil { + return nil + } step := &Node{ Step: s, Log: "", diff --git a/internal/models/status.go b/internal/models/status.go index bb7ad4f38..c0852d18e 100644 --- a/internal/models/status.go +++ b/internal/models/status.go @@ -77,18 +77,10 @@ func NewStatus(cfg *config.Config, nodes []*scheduler.Node, status scheduler.Sch models = FromSteps(cfg.Steps) } var onExit, onSuccess, onFailure, onCancel *Node = nil, nil, nil, nil - if cfg.HandlerOn.Exit != nil { - onExit = fromStepWithDefValues(cfg.HandlerOn.Exit) - } - if cfg.HandlerOn.Success != nil { - onSuccess = fromStepWithDefValues(cfg.HandlerOn.Success) - } - if cfg.HandlerOn.Failure != nil { - onFailure = fromStepWithDefValues(cfg.HandlerOn.Failure) - } - if cfg.HandlerOn.Cancel != nil { - onCancel = fromStepWithDefValues(cfg.HandlerOn.Cancel) - } + onExit = fromStepWithDefValues(cfg.HandlerOn.Exit) + onSuccess = fromStepWithDefValues(cfg.HandlerOn.Success) + onFailure = fromStepWithDefValues(cfg.HandlerOn.Failure) + onCancel = fromStepWithDefValues(cfg.HandlerOn.Cancel) return &Status{ RequestId: "", Name: cfg.Name, diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 5a615270a..7adf9de7f 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -21,22 +21,14 @@ func DefaultEnv() map[string]string { // MustGetUserHomeDir returns current working directory. // Panics is os.UserHomeDir() returns error func MustGetUserHomeDir() string { - hd, err := os.UserHomeDir() - if err != nil { - panic(err) - } - + hd, _ := os.UserHomeDir() return hd } // MustGetwd returns current working directory. // Panics is os.Getwd() returns error func MustGetwd() string { - wd, err := os.Getwd() - if err != nil { - panic(err) - } - + wd, _ := os.Getwd() return wd } @@ -52,11 +44,7 @@ func ParseTime(val string) (time.Time, error) { if val == constants.TimeEmpty { return time.Time{}, nil } - ret, err := time.ParseInLocation(constants.TimeFormat, val, time.Local) - if err != nil { - return time.Time{}, err - } - return ret, nil + return time.ParseInLocation(constants.TimeFormat, val, time.Local) } func FormatDuration(t time.Duration, defaultVal string) string { diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index c3412a7a8..c73bff17b 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/yohamta/dagu/internal/constants" "github.com/yohamta/dagu/internal/utils" ) @@ -25,6 +26,11 @@ func TestMustGetUserHomeDir(t *testing.T) { assert.Equal(t, "/test", hd) } +func TestDefaultEnv(t *testing.T) { + env := utils.DefaultEnv() + require.Contains(t, env, "PATH") +} + func TestMustGetwd(t *testing.T) { wd, _ := os.Getwd() assert.Equal(t, utils.MustGetwd(), wd) @@ -39,6 +45,10 @@ func TestFormatTime(t *testing.T) { require.NoError(t, err) assert.Equal(t, tm, parsed) + require.Equal(t, constants.TimeEmpty, utils.FormatTime(time.Time{})) + parsed, err = utils.ParseTime(constants.TimeEmpty) + require.NoError(t, err) + require.Equal(t, time.Time{}, parsed) } func TestFormatDuration(t *testing.T) { @@ -63,17 +73,47 @@ func TestValidFilename(t *testing.T) { assert.Equal(t, f, "file_name") } +func TestOpenFile(t *testing.T) { + tmp, err := ioutil.TempDir("", "open") + require.NoError(t, err) + + name := path.Join(tmp, "/file.txt") + f, err := os.OpenFile(name, os.O_CREATE|os.O_WRONLY, 0644) + require.NoError(t, err) + + defer func() { + f.Close() + os.Remove(name) + }() + + f.WriteString("test") + f.Sync() + f.Close() + + _, err = utils.OpenFile(name) + require.NoError(t, err) +} + func TestOpenOrCreateFile(t *testing.T) { - tmp, err := ioutil.TempDir("", "utils_test") + tmp, err := ioutil.TempDir("", "open_or_create") require.NoError(t, err) - name := path.Join(tmp, "/file_for_test.txt") + + name := path.Join(tmp, "/file.txt") f, err := utils.OpenOrCreateFile(name) require.NoError(t, err) + defer func() { f.Close() os.Remove(name) }() + require.True(t, utils.FileExists(name)) + + f.Close() + os.Remove(name) + + _, err = utils.OpenFile(name) + require.Error(t, err) } func TestParseVariable(t *testing.T) { diff --git a/tests/testdata/controller_config_error.yaml b/tests/testdata/controller_config_error.yaml new file mode 100644 index 000000000..945467f2b --- /dev/null +++ b/tests/testdata/controller_config_error.yaml @@ -0,0 +1,3 @@ +steps: + - name: "1" + command: "true" \ No newline at end of file diff --git a/tests/testdata/controller_update_status_failed.yaml b/tests/testdata/controller_update_status_failed.yaml new file mode 100644 index 000000000..433462d0b --- /dev/null +++ b/tests/testdata/controller_update_status_failed.yaml @@ -0,0 +1,4 @@ +name: "update status" +steps: + - name: "1" + command: "sleep 1" \ No newline at end of file