Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jaredoconnell committed Apr 16, 2024
1 parent 3dd60af commit 3cf14b6
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
8 changes: 4 additions & 4 deletions workflow/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,10 +322,10 @@ func applyLifecycleScopes(
for _, stage := range stepLifecycle.Stages {
prefix := "$.steps." + workflowStepID + "." + stage.ID + "."
// Apply inputs
// Example with stage starting: $.steps.wait_step.starting.inputs.
// Example with stage "starting": $.steps.wait_step.starting.inputs.
addInputNamespacedScopes(allNamespaces, stage, prefix+"inputs.")
// Apply outputs
// Example with stage outputs: $.steps.wait_step.outputs.outputs.success
// Example with stage "outputs": $.steps.wait_step.outputs.outputs.
addOutputNamespacedScopes(allNamespaces, stage, prefix+"outputs.")
}
}
Expand Down Expand Up @@ -387,8 +387,8 @@ func addScopesWithReferences(allNamespaces map[string]schema.Scope, scope schema
for propertyID, property := range rootObject.Properties() {
if property.Type().TypeID() == schema.TypeIDRef {
refProperty := property.Type().(schema.Ref)
if refProperty.Namespace() != schema.DEFAULT_NAMESPACE && refProperty.ObjectReady() {
// Found a resolved reference with an object that is not included in the scope. Add it to the map.
if refProperty.Namespace() != schema.DEFAULT_NAMESPACE {
// Found a reference to an object that is not included in the scope. Add it to the map.
var referencedObject any = refProperty.GetObject()
refObjectSchema := referencedObject.(*schema.ObjectSchema)
allNamespaces[prefix+"."+propertyID] = schema.NewScopeSchema(refObjectSchema)
Expand Down
38 changes: 34 additions & 4 deletions workflow/executor_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,36 @@ func TestAddInputNamespacedScopes(t *testing.T) {
}
}

func TestAddScopesWithMissingCache(t *testing.T) {
allNamespaces := make(map[string]schema.Scope)
externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "not-applied-namespace", nil)
notAppliedExternalRefProperty := schema.NewPropertySchema(
externalRef3,
nil,
true,
nil,
nil,
nil,
nil,
nil,
)
testScope := schema.NewScopeSchema(
schema.NewObjectSchema(
"scopeTestObjectA",
map[string]*schema.PropertySchema{
"notAppliedExternalRef": notAppliedExternalRefProperty,
},
),
)
assert.PanicsContains(
t,
func() {
addScopesWithReferences(allNamespaces, testScope, "$")
},
"scope with namespace \"not-applied-namespace\" was not applied successfully",
)
}

func TestAddScopesWithReferences(t *testing.T) {
// Test that the scope itself and the resolved references are added.
allNamespaces := make(map[string]schema.Scope)
Expand Down Expand Up @@ -121,9 +151,9 @@ func TestAddScopesWithReferences(t *testing.T) {
nil,
nil,
)
externalRef3 := schema.NewNamespacedRefSchema("scopeTestObjectC", "test-namespace-3", nil)
notAppliedExternalRefProperty := schema.NewPropertySchema(
externalRef3,
// This one shouldn't add a namespace.
nonRefProperty := schema.NewPropertySchema(
schema.NewStringSchema(nil, nil, nil),
nil,
true,
nil,
Expand All @@ -139,7 +169,7 @@ func TestAddScopesWithReferences(t *testing.T) {
"internalRef": internalRefProperty,
"appliedExternalRef": appliedExternalRefProperty,
"rootPrefixAppliedExternalRef": rootPrefixAppliedExternalRefProperty,
"notAppliedExternalRef": notAppliedExternalRefProperty,
"nonRefProperty": nonRefProperty,
},
),
)
Expand Down

0 comments on commit 3cf14b6

Please sign in to comment.