Add tests

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 {

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)

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)
 }
 

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)
+}

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
 }

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:        "",

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,

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 {

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) {

tests/testdata/controller_config_error.yaml

@@ -0,0 +1,3 @@
+steps:
+  - name: "1"
+    command: "true"