Repository: SimonBaeumer/commander (go)

Issue:
Running `commander add --file tests/commander.yaml my-command` removes test validation from commander.yaml
### Prerequisites

* [x] Can you reproduce the problem?
* [x] Are you running the latest version?
* [x] Did you perform a cursory search?

### Description

When using the automatic adding of tests, manually added tests validation is removed.

### Steps to Reproduce

1. run `commander add --file tests/commander.yaml ./setup-update-tag.sh`
2. check what has changed in the file (e.g. `diff --git a/tests/commander.yaml b/tests/commander.yaml`)

**Expected behavior:** `commander add` should just add new tests, not modify existing ones (resorting the file may be fine)

**Actual behavior:** It restructures the file and drops test cases. Basically removing most manually made edits.

```
diff --git a/tests/commander.yaml b/tests/commander.yaml
index 3156fb7..15b5d6c 100755
--- a/tests/commander.yaml
+++ b/tests/commander.yaml
@@ -1,33 +1,18 @@
 tests:
-  setup with existing .env:
-    command: ./setup.sh
+  ./setup-update-tag.sh:
     exit-code: 1
-    stdout: |-
-      Creating individual env files for containers (if they do not exist already)
-      .env already exists with initial configuration
-      If you want to change the configuration, please edit .env directly
+    stdout: Please install reg in order to run this script.
   ./version.sh:
     exit-code: 0
   ./version.sh core:
     exit-code: 0
-    stdout:
-      not-contains:
-        - core-
-        - Debian
-        - amd64
-        - tar.gz
   ./version.sh webapp:
     exit-code: 0
-    stdout:
-      not-contains:
-        - webapp-
-        - Debian
-        - amd64
-        - tar.gz
   ./version.sh zpush:
     exit-code: 0
-    stdout:
-      not-contains:
-        - Debian
-        - amd64
-        - tar.gz
+  setup with existing .env:
+    exit-code: 1
+    stdout: |-
+      Creating individual env files for containers (if they do not exist already)
+      .env already exists with initial configuration
+      If you want to change the configuration, please edit .env directly
```

### Specifications

 - Version: 1.12.0
 - Platform: Ubuntu 16.04
 - Subsystem: 
 
You can get the version information from executing `commander --version`.

Pull request description:
Fix add command for existing stdout and stderr properties
Fixes #86

## Checklist

 - [x] Added unit / integration tests for windows, macOS and Linux? 
 - [x] Added a changelog entry in [CHANGELOG.md](../CHANGELOG.md)?
 - [x] Updated the documentation ([README.md](../README.md), [docs](../docs))?
 - [x] Does your change work on `Linux`, `Windows` and `macOS`?

Patch:
diff --git a/.gitignore b/.gitignore
index 66e4733..97df132 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,4 +7,5 @@ tmp.yml
 *.out
 coverage
 *.exe
-commander.yaml
\ No newline at end of file
+commander.yaml
+tmp
\ No newline at end of file
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 873e8e4..620806e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,11 @@
+# v1.2.1
+
+ - Fix `add` command if `stdout` or `stderr` properties were removed if a new test was added
+
 # v1.2.0
 
-- Add reading envrionment variables from shell
-- Add `interval` option for `retries` which allows to execute a retry after a given period of time. I.e. `interval: 50ms`
+ - Add reading environment variables from shell, i.e. `${PATH}`
+ - Add `interval` option for `retries` which allows to execute a retry after a given period of time. I.e. `interval: 50ms`
 
 # v1.1.0
 
diff --git a/pkg/app/add_command.go b/pkg/app/add_command.go
index 13552b5..2377c0f 100644
--- a/pkg/app/add_command.go
+++ b/pkg/app/add_command.go
@@ -21,6 +21,7 @@ func AddCommand(command string, existed []byte) ([]byte, error) {
 		return []byte{}, err
 	}
 
+	//If a suite existed before adding the new command it is need to parse it and re-add it
 	if len(existed) > 0 {
 		err := yaml.UnmarshalStrict(existed, &conf)
 		if err != nil {
@@ -28,20 +29,27 @@ func AddCommand(command string, existed []byte) ([]byte, error) {
 		}
 
 		for k, t := range conf.Tests {
-			conf.Tests[k] = suite.YAMLTest{
+			test := suite.YAMLTest{
 				Title:    t.Title,
-				Stdout:   convertExpectedOut(t.Stdout.(runtime.ExpectedOut)),
-				Stderr:   convertExpectedOut(t.Stderr.(runtime.ExpectedOut)),
+				Stdout:   t.Stdout.(runtime.ExpectedOut),
+				Stderr:   t.Stderr.(runtime.ExpectedOut),
 				ExitCode: t.ExitCode,
 				Config:   convertConfig(t.Config),
 			}
+
+			//If title and command are not equal add the command property to the struct
+			if t.Title != t.Command {
+				test.Command = t.Command
+			}
+
+			conf.Tests[k] = test
 		}
 	}
 
 	conf.Tests[command] = suite.YAMLTest{
 		Title:    command,
-		Stdout:   stringOrNil(c.Stdout()),
-		Stderr:   stringOrNil(c.Stderr()),
+		Stdout:   runtime.ExpectedOut{Contains: []string{c.Stdout()}},
+		Stderr:   runtime.ExpectedOut{Contains: []string{c.Stderr()}},
 		ExitCode: c.ExitCode(),
 	}
 
@@ -53,26 +61,9 @@ func AddCommand(command string, existed []byte) ([]byte, error) {
 	return out, nil
 }
 
-func stringOrNil(str string) interface{} {
-	if str == "" {
-		return nil
-	}
-	return str
-}
-
 func convertConfig(config suite.YAMLTestConfig) suite.YAMLTestConfig {
 	if config.Dir == "" && len(config.Env) == 0 && config.Timeout == "" {
 		return suite.YAMLTestConfig{}
 	}
 	return config
 }
-
-func convertExpectedOut(out runtime.ExpectedOut) interface{} {
-	if len(out.Contains) == 1 && len(out.Lines) == 0 && out.Exactly == "" {
-		return out.Contains[0]
-	}
-	if len(out.Contains) == 0 {
-		return nil
-	}
-	return out
-}
diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go
index e3f9778..0dccfda 100644
--- a/pkg/runtime/runtime.go
+++ b/pkg/runtime/runtime.go
@@ -70,11 +70,11 @@ type Expected struct {
 
 //ExpectedOut represents the assertions on stdout and stderr
 type ExpectedOut struct {
-	Contains    []string
-	Lines       map[int]string
-	Exactly     string
-	LineCount   int
-	NotContains []string
+	Contains    []string       `yaml:"contains,omitempty"`
+	Lines       map[int]string `yaml:"lines,omitempty"`
+	Exactly     string         `yaml:"exactly,omitempty"`
+	LineCount   int            `yaml:"line-count,omitempty"`
+	NotContains []string       `yaml:"not-contains,omitempty"`
 }
 
 // CommandUnderTest represents the command under test
diff --git a/pkg/suite/yaml_suite.go b/pkg/suite/yaml_suite.go
index 1de03a4..9c6590a 100644
--- a/pkg/suite/yaml_suite.go
+++ b/pkg/suite/yaml_suite.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"github.com/SimonBaeumer/commander/pkg/runtime"
 	"gopkg.in/yaml.v2"
+	"reflect"
 	"strings"
 )
 
@@ -260,3 +261,55 @@ func (y *YAMLConfig) mergeEnvironmentVariables(global YAMLTestConfig, local YAML
 	}
 	return env
 }
+
+//MarshalYAML adds custom logic to the struct to yaml conversion
+func (y YAMLConfig) MarshalYAML() (interface{}, error) {
+	//Detect which values of the stdout/stderr assertions should be filled.
+	//If all values are empty except Contains it will convert it to a single string
+	//to match the easiest test suite definitions
+	for k, t := range y.Tests {
+		t.Stdout = convertExpectedOut(t.Stdout.(runtime.ExpectedOut))
+		if reflect.ValueOf(t.Stdout).Kind() == reflect.Struct {
+			t.Stdout = t.Stdout.(runtime.ExpectedOut)
+		}
+
+		t.Stderr = convertExpectedOut(t.Stderr.(runtime.ExpectedOut))
+		if reflect.ValueOf(t.Stderr).Kind() == reflect.Struct {
+			t.Stderr = t.Stderr.(runtime.ExpectedOut)
+		}
+
+		y.Tests[k] = t
+	}
+
+	return y, nil
+}
+
+func convertExpectedOut(out runtime.ExpectedOut) interface{} {
+	//If the property contains consists of only one element it will be set without the struct structure
+	if isContainsASingleNonEmptyString(out) && propertiesAreEmpty(out) {
+		return out.Contains[0]
+	}
+
+	//If the contains property only has one empty string element it should not be displayed
+	//in the marshaled yaml file
+	if len(out.Contains) == 1 && out.Contains[0] == "" {
+		out.Contains = nil
+	}
+
+	if len(out.Contains) == 0 && propertiesAreEmpty(out) {
+		return nil
+	}
+	return out
+}
+
+func propertiesAreEmpty(out runtime.ExpectedOut) bool {
+	return out.Lines == nil &&
+		out.Exactly == "" &&
+		out.LineCount == 0 &&
+		out.NotContains == nil
+}
+
+func isContainsASingleNonEmptyString(out runtime.ExpectedOut) bool {
+	return len(out.Contains) == 1 &&
+		out.Contains[0] != ""
+}


Test patch:
diff --git a/pkg/app/add_command_test.go b/pkg/app/add_command_test.go
index 2b81d7d..00078c2 100644
--- a/pkg/app/add_command_test.go
+++ b/pkg/app/add_command_test.go
@@ -24,3 +24,50 @@ tests:
 	assert.Nil(t, err)
 	assert.Equal(t, "tests:\n  echo exists:\n    exit-code: 0\n  echo hello:\n    exit-code: 0\n    stdout: hello\n", string(content))
 }
+
+func Test_AddCommand_AddToExistingWithComplexStdStreamAssertions(t *testing.T) {
+	existing := []byte(`
+tests:
+  exists:
+    command: echo exists
+    stdout:
+      contains:
+        - exists
+      not-contains:
+        - byebye
+    stderr:
+      not-contains:
+        - stderr not
+      line-count: 10
+      lines:
+        1: line1
+        2: line2
+    exit-code: 0
+`)
+
+	content, err := AddCommand("echo hello", existing)
+
+	expected := []byte(`tests:
+  echo hello:
+    exit-code: 0
+    stdout: hello
+  exists:
+    command: echo exists
+    exit-code: 0
+    stdout:
+      contains:
+      - exists
+      not-contains:
+      - byebye
+    stderr:
+      lines:
+        1: line1
+        2: line2
+      line-count: 10
+      not-contains:
+      - stderr not
+`)
+
+	assert.Nil(t, err)
+	assert.Equal(t, string(expected), string(content))
+}
diff --git a/pkg/suite/yaml_suite_test.go b/pkg/suite/yaml_suite_test.go
index e004f1f..928534b 100644
--- a/pkg/suite/yaml_suite_test.go
+++ b/pkg/suite/yaml_suite_test.go
@@ -248,3 +248,70 @@ tests:
 
 	_ = ParseYAML(yaml)
 }
+
+func Test_YAMLConfig_MarshalYAML(t *testing.T) {
+	conf := YAMLConfig{Tests: map[string]YAMLTest{
+		"return_string": {
+			Stdout: runtime.ExpectedOut{Contains: []string{"stdout string"}},
+			Stderr: runtime.ExpectedOut{Contains: []string{"stderr string"}},
+		},
+		"return_struct": {
+			Stdout: runtime.ExpectedOut{
+				Contains:  []string{"stdout"},
+				LineCount: 10,
+			},
+			Stderr: runtime.ExpectedOut{
+				Contains:  []string{"stderr"},
+				LineCount: 10,
+			},
+		},
+		"return_nil": {
+			Stdout: runtime.ExpectedOut{},
+			Stderr: runtime.ExpectedOut{},
+		},
+	}}
+
+	out, _ := conf.MarshalYAML()
+	r := out.(YAMLConfig)
+
+	assert.Equal(t, "stdout string", r.Tests["return_string"].Stdout)
+	assert.Equal(t, "stderr string", r.Tests["return_string"].Stderr)
+
+	assert.Equal(t, conf.Tests["return_struct"].Stdout, r.Tests["return_struct"].Stdout)
+	assert.Equal(t, conf.Tests["return_struct"].Stderr, r.Tests["return_struct"].Stderr)
+
+	assert.Nil(t, r.Tests["return_nil"].Stdout)
+	assert.Nil(t, r.Tests["return_nil"].Stderr)
+}
+
+func Test_convertExpectOut_ReturnNilIfEmpty(t *testing.T) {
+	out := runtime.ExpectedOut{
+		Contains: []string{""},
+	}
+
+	r := convertExpectedOut(out)
+
+	assert.Nil(t, r)
+}
+
+func Test_convertExpectedOut_ReturnContainsAsString(t *testing.T) {
+	out := runtime.ExpectedOut{
+		Contains: []string{"test"},
+	}
+
+	r := convertExpectedOut(out)
+
+	assert.Equal(t, "test", r)
+}
+
+func Test_convertExpectedOut_ReturnFullStruct(t *testing.T) {
+	out := runtime.ExpectedOut{
+		Contains:  []string{"hello", "hi"},
+		LineCount: 10,
+		Exactly:   "test",
+	}
+
+	r := convertExpectedOut(out)
+
+	assert.Equal(t, out, r)
+}