From 54a73f3e0744527100d1ac3943d043a3bbadc73c Mon Sep 17 00:00:00 2001
From: Matthew F Leader <mleader@redhat.com>
Date: Tue, 5 Dec 2023 13:14:40 -0500
Subject: [PATCH] add comment to test

---
 workflow/executor.go      | 34 +++++++++++++++++++---------------
 workflow/executor_test.go |  5 +++++
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/workflow/executor.go b/workflow/executor.go
index 74c89542..ec6d7153 100644
--- a/workflow/executor.go
+++ b/workflow/executor.go
@@ -556,18 +556,32 @@ func (e *executor) buildOutputProperties(
 		if stageOutputsLen > 0 {
 			// only add stages with outputs
 			stageOutputProperties := make(map[string]*schema.PropertySchema, stageOutputsLen)
-			m, err2 := e.addOutputProperties(
+			stageOutputs, err2 := e.addOutputProperties(
 				stage, stepID, runnableStep, dag, stepNode,
-				stageOutputProperties, outputProperties)
+				stageOutputProperties)
 			if err2 != nil {
-				return m, err2
+				return nil, err2
 			}
+			outputProperties[stage.ID] = schema.NewPropertySchema(
+				stageOutputs,
+				nil,
+				true,
+				nil,
+				nil,
+				nil,
+				nil,
+				nil,
+			)
 		}
 	}
 	return outputProperties, nil
 }
 
-func (e *executor) addOutputProperties(stage step.LifecycleStageWithSchema, stepID string, runnableStep step.RunnableStep, dag dgraph.DirectedGraph[*DAGItem], stepNode dgraph.Node[*DAGItem], stageOutputProperties map[string]*schema.PropertySchema, outputProperties map[string]*schema.PropertySchema) (map[string]*schema.PropertySchema, error) {
+func (e *executor) addOutputProperties(
+	stage step.LifecycleStageWithSchema, stepID string, runnableStep step.RunnableStep,
+	dag dgraph.DirectedGraph[*DAGItem], stepNode dgraph.Node[*DAGItem],
+	stageOutputProperties map[string]*schema.PropertySchema) (*schema.ObjectSchema, error) {
+
 	for outputID, outputSchema := range stage.Outputs {
 		stageDAGItem := &DAGItem{
 			Kind:     DAGItemKindStepStageOutput,
@@ -609,17 +623,7 @@ func (e *executor) addOutputProperties(stage step.LifecycleStageWithSchema, step
 		stageOutputProperties,
 	)
 
-	outputProperties[stage.ID] = schema.NewPropertySchema(
-		stageOutputs,
-		nil,
-		true,
-		nil,
-		nil,
-		nil,
-		nil,
-		nil,
-	)
-	return nil, nil
+	return stageOutputs, nil
 }
 
 func (e *executor) getRunData(stepKind step.Provider, runnableStep step.RunnableStep, stepID string, stepDataMap map[any]any) (map[string]any, error) {
diff --git a/workflow/executor_test.go b/workflow/executor_test.go
index 165c5f28..86160ba5 100644
--- a/workflow/executor_test.go
+++ b/workflow/executor_test.go
@@ -269,9 +269,14 @@ outputs:
 func TestDependOnNoOutputs(t *testing.T) {
 	// This test is to validate that this error is caught at workflow
 	// preparation instead of workflow execution.
+
 	// The error handling does not currently distinguish between the edge cases:
 	// - wait_1 = {}; not having a property named 'deploy',
 	// - wait_1 = { deploy: nil }; the 'deploy' property has no outputs (i.e. nil output)
+	//
+	// This is not a robust test. It should be improved, or removed, if it continues to break.
+	// To improve this test the engine needs to improve observability
+	// into the workflow's path structure at preparation time.
 	_, err := getTestImplPreparedWorkflow(t, invalidWaitfor)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "object wait_1 does not have a property named deploy")