Comprehensively Refactor Step Structure (#382)

* Simplify step creation by refactoring shared code and streamline unmarshalling
* Use the simplified setup to add new step types print_str and remove_path

.pre-commit-config.yaml

@@ -22,7 +22,7 @@ repos:
     rev: v3.0.3
     hooks:
       - id: prettier
-        files: \.(json|yaml|yml)$
+        files: \.(json)$
         exclude: docs/container.md
 
   - repo: https://github.com/jumanjihouse/pre-commit-hooks
@@ -84,13 +84,6 @@ repos:
         entry: .hooks/go-vet.sh
         files: '\.go$'
 
-      - id: go-critic
-        name: go-critic
-        language: script
-        files: '\.go$'
-        entry: .hooks/go-critic.sh
-        args: ["check"]
-
       - id: go-licenses
         name: Run go-licenses
         language: script

README.md

@@ -21,7 +21,6 @@ targets and mediums.
 - [Using the TTPForge Dev Container](docs/container.md)
 - [Code Standards](docs/code-standards.md)
 - [Creating a new release](docs/release.md)
-- [TTPForge Building Blocks](docs/building-blocks.md)
 
 ---
 

cmd/run.go

@@ -51,12 +51,12 @@ func buildRunCommand(cfg *Config) *cobra.Command {
 			// based on the TTPs argument value specifications
 			ttpCfg.Repo = foundRepo
 
-			ttp, err := blocks.LoadTTP(ttpAbsPath, foundRepo.GetFs(), &ttpCfg, argsList)
+			ttp, execCtx, err := blocks.LoadTTP(ttpAbsPath, foundRepo.GetFs(), &ttpCfg, argsList)
 			if err != nil {
 				return fmt.Errorf("could not load TTP at %v:\n\t%v", ttpAbsPath, err)
 			}
 
-			if _, err := ttp.RunSteps(ttpCfg); err != nil {
+			if _, err := ttp.Execute(execCtx); err != nil {
 				return fmt.Errorf("failed to run TTP at %v: %v", ttpAbsPath, err)
 			}
 			return nil

cmd/run_test.go

@@ -22,12 +22,9 @@ package cmd_test
 import (
 	"bytes"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/facebookincubator/ttpforge/cmd"
-	"github.com/facebookincubator/ttpforge/pkg/blocks"
-	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -70,7 +67,18 @@ func TestRun(t *testing.T) {
 				testConfigFilePath,
 				"another-repo//simple-inline.yaml",
 			},
-			expectedStdout: "simple inline was executed\n",
+			expectedStdout: "simple inline was executed\ncleaning up simple inline\n",
+		},
+		{
+			name:        "subttp-cleanup",
+			description: "verify that execution of a subTTP with cleanup succeeds",
+			args: []string{
+				"-c",
+				testConfigFilePath,
+				"another-repo//sub-ttp-example/ttp.yaml",
+			},
+			expectedStdout: "subttp1_step_1\nsubttp1_step_2\nsubttp2_step_1\nsubttp2_step_1_cleanup\nsubttp1_step_2_cleanup\nsubttp1_step_1_cleanup\n",
+			wantError:      true,
 		},
 		{
 			name:        "dry-run-success",
@@ -94,6 +102,27 @@ func TestRun(t *testing.T) {
 			},
 			wantError: true,
 		},
+		{
+			name:        "no-cleanup",
+			description: "Using the no-cleanup flag should prevent cleanup",
+			args: []string{
+				"-c",
+				testConfigFilePath,
+				"--no-cleanup",
+				"another-repo//simple-inline.yaml",
+			},
+			expectedStdout: "simple inline was executed\n",
+		},
+		{
+			name:        "cleanup-stress-test",
+			description: "run many different execute+cleanup combinations",
+			args: []string{
+				"-c",
+				testConfigFilePath,
+				"another-repo//cleanup-tests/stress-tests.yaml",
+			},
+			expectedStdout: "execute_step_1\nexecute_step_2\nexecute_step_3\nexecute_step_4\ncleanup_step_4\ncleanup_step_3\ncleanup_step_2\ncleanup_step_1\n",
+		},
 	}
 
 	for _, tc := range testCases {
@@ -114,89 +143,3 @@ func TestRun(t *testing.T) {
 		})
 	}
 }
-
-func TestNoCleanupFlag(t *testing.T) {
-	afs := afero.NewOsFs()
-	testCases := []struct {
-		name             string
-		content          string
-		execConfig       blocks.TTPExecutionConfig
-		expectedDirExist bool
-		wantError        bool
-	}{
-		{
-			name: "Test No Cleanup Behavior - Directory Creation",
-			content: `
----
-name: test-cleanup
-steps:
-  - name: step_one
-    inline: mkdir testDir
-    cleanup:
-      inline: rm -rf testDir`,
-			execConfig: blocks.TTPExecutionConfig{
-				NoCleanup: true,
-			},
-			expectedDirExist: true,
-			wantError:        false,
-		},
-		{
-			name: "Test Cleanup Behavior - Directory Deletion",
-			content: `
----
-name: test-cleanup-2
-steps:
-  - name: step_two
-    inline: mkdir testDir2
-    cleanup:
-      inline: rm -rf testDir2`,
-			execConfig: blocks.TTPExecutionConfig{
-				NoCleanup: false,
-			},
-			expectedDirExist: false,
-			wantError:        false,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			// Create a temp directory to work within
-			tempDir, err := afero.TempDir(afs, "", "testCleanup")
-			require.NoError(t, err)
-
-			// Update content to work within the temp directory
-			tc.content = strings.ReplaceAll(tc.content, "mkdir ", "mkdir "+tempDir+"/")
-			tc.content = strings.ReplaceAll(tc.content, "rm -rf ", "rm -rf "+tempDir+"/")
-
-			// Render the templated TTP first
-			ttp, err := blocks.RenderTemplatedTTP(tc.content, &tc.execConfig)
-			require.NoError(t, err)
-
-			// Handle potential error from RemoveAll within a deferred function
-			defer func() {
-				err := afs.RemoveAll(tempDir) // cleanup temp directory
-				if err != nil {
-					t.Errorf("failed to remove temp directory: %v", err)
-				}
-			}()
-
-			_, err = ttp.RunSteps(tc.execConfig)
-			if tc.wantError {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
-
-			// Determine which directory to check based on the test case content
-			dirName := tempDir + "/testDir"
-			if strings.Contains(tc.content, "testDir2") {
-				dirName = tempDir + "/testDir2"
-			}
-
-			// Check if the directory exists
-			dirExists, err := afero.DirExists(afs, dirName)
-			require.NoError(t, err)
-			assert.Equal(t, tc.expectedDirExist, dirExists)
-		})
-	}
-}

cmd/test-resources/repos/another-repo/some-ttps/cleanup-tests/stress-tests.yaml

@@ -0,0 +1,30 @@
+---
+name: cleanup-stress-test
+description: |
+  Cleanup a whole bunch of different ways
+steps:
+  - name: file-step-with-inline-cleanup
+    file: test.sh
+    args:
+      - execute_step_1
+    cleanup:
+      inline: |
+        echo "cleanup_step_1"
+  - name: print-with-file-cleanup
+    print_str: execute_step_2
+    cleanup:
+      file: test.sh
+      args:
+        - cleanup_step_2
+  - name: ttp-with-print-cleanup
+    ttp: cleanup-tests/subttp/ttp.yaml
+    args:
+      to_print: execute_step_3
+    cleanup:
+      print_str: cleanup_step_3
+  - name: inline-with-ttp-cleanup
+    inline: echo execute_step_4
+    cleanup:
+      ttp: cleanup-tests/subttp/ttp.yaml
+      args:
+        to_print: cleanup_step_4

cmd/test-resources/repos/another-repo/some-ttps/cleanup-tests/subttp/ttp.yaml

@@ -0,0 +1,7 @@
+---
+name: print-stuff
+args:
+  - name: to_print
+steps:
+  - name: print-arg
+    print_str: {{.Args.to_print}}

cmd/test-resources/repos/another-repo/some-ttps/cleanup-tests/test.sh

@@ -0,0 +1,2 @@
+#!/bin/bash
+echo "$1"

cmd/test-resources/repos/another-repo/some-ttps/simple-inline.yaml

@@ -3,3 +3,5 @@ name: basic_inline
 steps:
   - name: hello
     inline: echo "simple inline was executed"
+    cleanup:
+      inline: echo "cleaning up simple inline"

cmd/test-resources/repos/another-repo/some-ttps/sub-ttp-example/subttp1.yaml

@@ -0,0 +1,11 @@
+---
+name: subttp1
+steps:
+  - name: subttp1_step_1
+    inline: echo subttp1_step_1
+    cleanup:
+      inline: echo subttp1_step_1_cleanup
+  - name: subttp1_step_2
+    inline: echo subttp1_step_2
+    cleanup:
+      inline: echo subttp1_step_2_cleanup

cmd/test-resources/repos/another-repo/some-ttps/sub-ttp-example/subttp2.yaml

@@ -0,0 +1,11 @@
+---
+name: subttp2
+steps:
+  - name: subttp2_step_1
+    inline: echo subttp2_step_1
+    cleanup:
+      inline: echo subttp2_step_1_cleanup
+  - name: intentional_failure
+    inline: foobarbaz
+    cleanup:
+      inline: echo subttp2_step_2_cleanup

cmd/test-resources/repos/another-repo/some-ttps/sub-ttp-example/ttp.yaml

@@ -0,0 +1,7 @@
+---
+name: subttp_cleanup_test
+steps:
+  - name: first_sub_ttp
+    ttp: sub-ttp-example/subttp1.yaml
+  - name: second_sub_ttp
+    ttp: sub-ttp-example/subttp2.yaml