From 6be6787f8d1e19bc5ab6704b9263303abd144a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillaume=20Cor=C3=A9?= Date: Fri, 25 Aug 2023 13:23:49 +0200 Subject: [PATCH] Fix order of the inner Include List inside a file (#33) * 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 --- cli/agnosticv.go | 6 +-- cli/agnosticv_test.go | 6 +-- cli/fixtures/includes/order1.yaml | 3 ++ cli/fixtures/includes/order2.yaml | 6 +++ cli/fixtures/includes/order21.yaml | 2 + cli/fixtures/includes/order22.yaml | 1 + cli/fixtures/includes/order23.yaml | 1 + cli/fixtures/includes/order3.yaml | 3 ++ cli/fixtures/test/foo/order.yaml | 5 +++ cli/includes.go | 2 +- cli/merge_test.go | 62 +++++++++++++++++++++++++++++- 11 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 cli/fixtures/includes/order1.yaml create mode 100644 cli/fixtures/includes/order2.yaml create mode 100644 cli/fixtures/includes/order21.yaml create mode 100644 cli/fixtures/includes/order22.yaml create mode 100644 cli/fixtures/includes/order23.yaml create mode 100644 cli/fixtures/includes/order3.yaml create mode 100644 cli/fixtures/test/foo/order.yaml diff --git a/cli/agnosticv.go b/cli/agnosticv.go index 8d187ae..16d9b25 100644 --- a/cli/agnosticv.go +++ b/cli/agnosticv.go @@ -85,7 +85,7 @@ 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: @@ -93,9 +93,9 @@ Examples: # 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") diff --git a/cli/agnosticv_test.go b/cli/agnosticv_test.go index d23dba3..d2e1a39 100644 --- a/cli/agnosticv_test.go +++ b/cli/agnosticv_test.go @@ -262,7 +262,7 @@ func TestWalk(t *testing.T) { { description: "No JMES filtering", hasFlags: []string{}, - count: 12, + count: 13, }, { description: "Related includes/include1.yaml", @@ -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", @@ -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", diff --git a/cli/fixtures/includes/order1.yaml b/cli/fixtures/includes/order1.yaml new file mode 100644 index 0000000..97d18f7 --- /dev/null +++ b/cli/fixtures/includes/order1.yaml @@ -0,0 +1,3 @@ +--- +foo: include1 +bar: include1 diff --git a/cli/fixtures/includes/order2.yaml b/cli/fixtures/includes/order2.yaml new file mode 100644 index 0000000..c41bbf2 --- /dev/null +++ b/cli/fixtures/includes/order2.yaml @@ -0,0 +1,6 @@ +--- +#include order21.yaml +#include order22.yaml +#include order23.yaml +foo: include2 +bar: include2 diff --git a/cli/fixtures/includes/order21.yaml b/cli/fixtures/includes/order21.yaml new file mode 100644 index 0000000..012506a --- /dev/null +++ b/cli/fixtures/includes/order21.yaml @@ -0,0 +1,2 @@ +foo: include21 +bar: include21 diff --git a/cli/fixtures/includes/order22.yaml b/cli/fixtures/includes/order22.yaml new file mode 100644 index 0000000..928697f --- /dev/null +++ b/cli/fixtures/includes/order22.yaml @@ -0,0 +1 @@ +bar: include22 diff --git a/cli/fixtures/includes/order23.yaml b/cli/fixtures/includes/order23.yaml new file mode 100644 index 0000000..b97ce28 --- /dev/null +++ b/cli/fixtures/includes/order23.yaml @@ -0,0 +1 @@ +bar: include23 diff --git a/cli/fixtures/includes/order3.yaml b/cli/fixtures/includes/order3.yaml new file mode 100644 index 0000000..c779670 --- /dev/null +++ b/cli/fixtures/includes/order3.yaml @@ -0,0 +1,3 @@ +--- +foo: include3 +bar: include3 diff --git a/cli/fixtures/test/foo/order.yaml b/cli/fixtures/test/foo/order.yaml new file mode 100644 index 0000000..f56281a --- /dev/null +++ b/cli/fixtures/test/foo/order.yaml @@ -0,0 +1,5 @@ +--- +#include ../../includes/order1.yaml +#include ../../includes/order2.yaml +#include ../../includes/order3.yaml +bar: order.yaml diff --git a/cli/includes.go b/cli/includes.go index de62f51..ee588da 100644 --- a/cli/includes.go +++ b/cli/includes.go @@ -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 diff --git a/cli/merge_test.go b/cli/merge_test.go index a758083..c0c0cee 100644 --- a/cli/merge_test.go +++ b/cli/merge_test.go @@ -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") @@ -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 { @@ -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) {