Skip to content

Commit

Permalink
Fix order of the inner Include List inside a file (#33)
Browse files Browse the repository at this point in the history
* Fix order of the inner Include List inside a file

jira: GPTEINFRA-7360

Inside a file, order of includes should be followed.

This change includes:

* Fix the parseAllIncludes function
* Add tests to ensure order of innerIncludes are correct

* Fix tests + go fmt
  • Loading branch information
fridim authored Aug 25, 2023
1 parent bd10b1b commit 6be6787
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 8 deletions.
6 changes: 3 additions & 3 deletions cli/agnosticv.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ Example:
List all catalog items under dir/ and also all catalog items that include includes/foo.yaml
Can be used several times (act like OR).`)
const hasHelp ="Use with --list only. Filter catalog items using a JMESPath `expression`."+`
const hasHelp = "Use with --list only. Filter catalog items using a JMESPath `expression`." + `
Can be used several times (act like AND).
Examples:
--has __meta__.catalog
# Compare a variable value
--has "env_type == 'ocp-clientvm'"
# Compare a variable numeric value
--has 'worker_instance_count == `+"`2`"+`'
--has 'worker_instance_count == ` + "`2`" + `'
# List all catalog items with a secret named 'gpte-sandbox'
--has 'length(__meta__.secrets[?name=='\''gpte-sandbox'\'']) > `+"`0`"+`'
--has 'length(__meta__.secrets[?name=='\''gpte-sandbox'\'']) > ` + "`0`" + `'
`
flags.Var(&hasFlags, "has", hasHelp)
flags.BoolVar(&debugFlag, "debug", false, "Debug mode")
Expand Down
6 changes: 3 additions & 3 deletions cli/agnosticv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ func TestWalk(t *testing.T) {
{
description: "No JMES filtering",
hasFlags: []string{},
count: 12,
count: 13,
},
{
description: "Related includes/include1.yaml",
Expand Down Expand Up @@ -298,7 +298,7 @@ func TestWalk(t *testing.T) {
hasFlags: []string{},
relatedFlags: []string{"includes/include1.yaml"},
orRelatedFlags: []string{"common.yaml"},
count: 12,
count: 13,
},
{
description: "Related (exclusive + inclusive) to /common.yaml and --has flag",
Expand All @@ -320,7 +320,7 @@ func TestWalk(t *testing.T) {
{
description: "Is a Babylon catalog item",
hasFlags: []string{"__meta__.catalog"},
count: 12,
count: 13,
},
{
description: "env_type is clientvm and purpose is development",
Expand Down
3 changes: 3 additions & 0 deletions cli/fixtures/includes/order1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
foo: include1
bar: include1
6 changes: 6 additions & 0 deletions cli/fixtures/includes/order2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
#include order21.yaml
#include order22.yaml
#include order23.yaml
foo: include2
bar: include2
2 changes: 2 additions & 0 deletions cli/fixtures/includes/order21.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
foo: include21
bar: include21
1 change: 1 addition & 0 deletions cli/fixtures/includes/order22.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bar: include22
1 change: 1 addition & 0 deletions cli/fixtures/includes/order23.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bar: include23
3 changes: 3 additions & 0 deletions cli/fixtures/includes/order3.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
foo: include3
bar: include3
5 changes: 5 additions & 0 deletions cli/fixtures/test/foo/order.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
#include ../../includes/order1.yaml
#include ../../includes/order2.yaml
#include ../../includes/order3.yaml
bar: order.yaml
2 changes: 1 addition & 1 deletion cli/includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func parseAllIncludes(path string, done map[string]bool) ([]Include, map[string]
}

innerIncludes = append(innerIncludes, include)
result = append(innerIncludes, result...)
result = append(result, innerIncludes...)
}
}
return result, done, nil
Expand Down
62 changes: 61 additions & 1 deletion cli/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,39 @@ func TestMergeCatalogItemIncluded(t *testing.T) {
t.Error("description is not 'from description.adoc'", value)
}
}

func TestMergeCatalogItemIncludedWithOrder(t *testing.T) {
initLoggers()
rootFlag = abs("fixtures")
initConf(rootFlag)
initSchemaList()
initMergeStrategies()
validateFlag = true
merged, _, err := mergeVars(
"fixtures/test/foo/order.yaml",
mergeStrategies,
)
if err != nil {
t.Fatal(err)
}

_, value, _, err := Get(merged, "/foo")
if err != nil {
t.Error(err)
}
if value != "include3" {
t.Error("foo should be 'include3'", value)
}

_, value, _, err = Get(merged, "/bar")
if err != nil {
t.Error(err)
}
if value != "order.yaml" {
t.Error("bar should be 'order.yaml'", value)
}
}

func TestMerge(t *testing.T) {
initLoggers()
rootFlag = abs("fixtures")
Expand Down Expand Up @@ -256,10 +289,10 @@ func TestMerge(t *testing.T) {
"/common.yaml",
"/test/account.yaml",
"/test/BABYLON_EMPTY_CONFIG_AWS/common.yaml",
"/includes/include2.meta.yml",
"/includes/include1.meta.yaml",
"/includes/include1.yaml",
"/test/BABYLON_EMPTY_CONFIG_AWS/test.meta.yaml",
"/includes/include2.meta.yml",
"/test/BABYLON_EMPTY_CONFIG_AWS/test.yaml",
}
for i, v := range expectedMergeList {
Expand All @@ -283,6 +316,33 @@ func TestMerge(t *testing.T) {
if !found || err != nil || value != "value1" {
t.Error("/__meta__/from_include1_meta be merged from detected meta")
}

_, includeList, err = mergeVars(
"fixtures/test/foo/order.yaml",
mergeStrategies,
)
if err != nil {
t.Fatal(err)
}

expectedMergeList = []string{
"/common.yaml",
"/test/account.yaml",
"/includes/order1.yaml",
"/includes/order21.yaml",
"/includes/order22.yaml",
"/includes/order23.yaml",
"/includes/order2.yaml",
"/includes/order3.yaml",
"/test/foo/order.yaml",
}
for i, v := range expectedMergeList {
if !strings.HasSuffix(includeList[i].path, v) {
t.Error(v, "not at the position", i, "in the merge list of ",
"fixtures/test/foo/order.yaml",
"found", includeList[i].path, "instead")
}
}
}

func TestMergeStrategyOverwrite(t *testing.T) {
Expand Down

0 comments on commit 6be6787

Please sign in to comment.