TTPForge StepVars Bugfix: Fix newline appended to stdout (#544)

Summary:
Pull Request resolved: #544

In inline blocks, when we capture stdout to a variable, we include the newline character used to move the cursor to a fresh line.  While this is technically correct, most users would expect something like `echo "asdf"` to load the variable with `asdf`, not `asdf\n`.

To fix this, I've opted just to remove a single newline from the end of stdout before loading it into the variable.

Reviewed By: d0n601

Differential Revision: D78497399

fbshipit-source-id: 41ab4c783f2d6ec0ec8d7754a58c6feb4f1f7c7e

Author: RoboticPrism

pkg/blocks/basicstep.go

@@ -24,6 +24,7 @@ import (
 	"errors"
 	"fmt"
 	"os/exec"
+	"strings"
 	"time"
 
 	"github.com/facebookincubator/ttpforge/pkg/logging"
@@ -120,7 +121,7 @@ func (b *BasicStep) Execute(execCtx TTPExecutionContext) (*ActResult, error) {
 	}
 	// Send stdout to the output variable
 	if b.OutputVar != "" {
-		execCtx.Vars.StepVars[b.OutputVar] = result.Stdout
+		execCtx.Vars.StepVars[b.OutputVar] = strings.TrimSuffix(result.Stdout, "\n")
 	}
 	return result, nil
 }

pkg/blocks/basicstep_test.go

@@ -99,7 +99,7 @@ outputs:
 
 func TestBasicStepExecuteWithTemplate(t *testing.T) {
 	content := `name: test_basic_step
-inline: echo -n "this is {[{.StepVars.foo}]}"`
+inline: echo "this is {[{.StepVars.foo}]}"`
 	var s BasicStep
 	execCtx := NewTTPExecutionContext()
 	execCtx.Vars.StepVars = map[string]string{
@@ -113,7 +113,7 @@ inline: echo -n "this is {[{.StepVars.foo}]}"`
 	require.NoError(t, err)
 	result, err := s.Execute(execCtx)
 	require.NoError(t, err)
-	assert.Equal(t, "this is successfully templated", result.Stdout, "stdout should be templated")
+	assert.Equal(t, "this is successfully templated\n", result.Stdout, "stdout should be templated")
 }
 
 func TestBasicStepRaisesErrorWithMissingTemplateVariable(t *testing.T) {
@@ -131,7 +131,7 @@ inline: echo "this is {[{.StepVars.foo}]}"`
 
 func TestBasicStepExecuteOutputsToOutputVar(t *testing.T) {
 	content := `name: test_basic_step
-inline: echo -n "bar"
+inline: echo "bar"
 outputvar: foo`
 	var s BasicStep
 	execCtx := NewTTPExecutionContext()
@@ -146,9 +146,26 @@ outputvar: foo`
 	assert.Equal(t, "bar", execCtx.Vars.StepVars["foo"], "outputvar should be set")
 }
 
+func TestBasicStepExecuteOutputsMultilineToOutputVar(t *testing.T) {
+	content := `name: test_basic_step
+inline: echo -e "line1\nline2\n\nline4\n"
+outputvar: foo`
+	var s BasicStep
+	execCtx := NewTTPExecutionContext()
+	err := yaml.Unmarshal([]byte(content), &s)
+	require.NoError(t, err)
+	err = s.Validate(execCtx)
+	require.NoError(t, err)
+	err = s.Template(execCtx)
+	require.NoError(t, err)
+	_, err = s.Execute(execCtx)
+	require.NoError(t, err)
+	assert.Equal(t, "line1\nline2\n\nline4\n", execCtx.Vars.StepVars["foo"], "outputvar should be set")
+}
+
 func TestBasicStepExecuteOutputsToAndOverwritesOutputVar(t *testing.T) {
 	content := `name: test_basic_step
-inline: echo -n "bar"
+inline: echo "bar"
 outputvar: foo`
 	var s BasicStep
 	execCtx := NewTTPExecutionContext()