From 4c7b57a1a8dea6c9f49c1597abd3d4a7ed9cb230 Mon Sep 17 00:00:00 2001 From: Roi Martin Date: Thu, 23 Jan 2025 17:37:55 +0100 Subject: [PATCH] internal/config: fix inconsistencies in tests --- internal/config/config_test.go | 10 ++-- internal/config/configgraph_test.go | 54 +++++++++---------- internal/config/dag/dag_test.go | 42 +++++---------- internal/config/merge_test.go | 14 ++--- .../{includes => include}/autoinclude.yaml | 2 +- internal/config/testdata/include/common.yaml | 9 ++++ .../include_a.yaml => include/common_a.yaml} | 2 +- .../include_b.yaml => include/common_b.yaml} | 2 +- .../config/testdata/include/duplicated.yaml | 9 ++++ .../include_local.yaml => include/local.yaml} | 2 +- .../no_includes.yaml} | 0 .../testdata/includes/include_duplicated.yaml | 9 ---- .../include_two_with_common_include.yaml | 9 ---- 13 files changed, 75 insertions(+), 89 deletions(-) rename internal/config/testdata/{includes => include}/autoinclude.yaml (74%) create mode 100644 internal/config/testdata/include/common.yaml rename internal/config/testdata/{includes/include_a.yaml => include/common_a.yaml} (71%) rename internal/config/testdata/{includes/include_b.yaml => include/common_b.yaml} (71%) create mode 100644 internal/config/testdata/include/duplicated.yaml rename internal/config/testdata/{includes/include_local.yaml => include/local.yaml} (71%) rename internal/config/testdata/{includes/without_includes.yaml => include/no_includes.yaml} (100%) delete mode 100644 internal/config/testdata/includes/include_duplicated.yaml delete mode 100644 internal/config/testdata/includes/include_two_with_common_include.yaml diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9a9d630..cb7eacf 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -16,7 +16,7 @@ import ( "github.com/google/go-cmp/cmp" ) -func TestParseFile(t *testing.T) { +func TestParse(t *testing.T) { tests := []struct { name string file string @@ -406,10 +406,6 @@ func TestSeverity_MarshalText(t *testing.T) { } } -func ptr[V any](v V) *V { - return &v -} - func TestParseExpirationDate(t *testing.T) { tests := []struct { name string @@ -463,3 +459,7 @@ func mustParseExpDate(date string) ExpirationDate { } return ExpirationDate{Time: t} } + +func ptr[V any](v V) *V { + return &v +} diff --git a/internal/config/configgraph_test.go b/internal/config/configgraph_test.go index b15cafb..6583bdb 100644 --- a/internal/config/configgraph_test.go +++ b/internal/config/configgraph_test.go @@ -12,7 +12,7 @@ import ( "github.com/adevinta/lava/internal/config/dag" ) -func TestResolver_NewDAG(t *testing.T) { +func TestNewConfigGraph(t *testing.T) { tests := []struct { name string URL string @@ -20,11 +20,11 @@ func TestResolver_NewDAG(t *testing.T) { wantErr bool }{ { - name: "File without includes", - URL: "testdata/includes/without_includes.yaml", + name: "no includes", + URL: "testdata/include/no_includes.yaml", want: ConfigGraph{ configs: map[string]Config{ - "testdata/includes/without_includes.yaml": { + "testdata/include/no_includes.yaml": { LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -41,12 +41,12 @@ func TestResolver_NewDAG(t *testing.T) { wantErr: false, }, { - name: "Include a local file", - URL: "testdata/includes/include_local.yaml", + name: "local file", + URL: "testdata/include/local.yaml", want: ConfigGraph{ configs: map[string]Config{ - "testdata/includes/include_local.yaml": { - Includes: []string{"testdata/includes/without_includes.yaml"}, + "testdata/include/local.yaml": { + Includes: []string{"testdata/include/no_includes.yaml"}, LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -58,7 +58,7 @@ func TestResolver_NewDAG(t *testing.T) { }, }, }, - "testdata/includes/without_includes.yaml": { + "testdata/include/no_includes.yaml": { LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -75,19 +75,19 @@ func TestResolver_NewDAG(t *testing.T) { wantErr: false, }, { - name: "Import itself", - URL: "testdata/includes/autoinclude.yaml", + name: "autoinclude", + URL: "testdata/include/autoinclude.yaml", wantErr: true, }, { - name: "Include duplicated", - URL: "testdata/includes/include_duplicated.yaml", + name: "duplicated", + URL: "testdata/include/duplicated.yaml", want: ConfigGraph{ configs: map[string]Config{ - "testdata/includes/include_duplicated.yaml": { + "testdata/include/duplicated.yaml": { Includes: []string{ - "testdata/includes/without_includes.yaml", - "testdata/includes/without_includes.yaml", + "testdata/include/no_includes.yaml", + "testdata/include/no_includes.yaml", }, LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ @@ -100,7 +100,7 @@ func TestResolver_NewDAG(t *testing.T) { }, }, }, - "testdata/includes/without_includes.yaml": { + "testdata/include/no_includes.yaml": { LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -117,14 +117,14 @@ func TestResolver_NewDAG(t *testing.T) { wantErr: false, }, { - name: "Include two with common include", - URL: "testdata/includes/include_two_with_common_include.yaml", + name: "common includes", + URL: "testdata/include/common.yaml", want: ConfigGraph{ configs: map[string]Config{ - "testdata/includes/include_two_with_common_include.yaml": { + "testdata/include/common.yaml": { Includes: []string{ - "testdata/includes/include_a.yaml", - "testdata/includes/include_b.yaml", + "testdata/include/common_a.yaml", + "testdata/include/common_b.yaml", }, LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ @@ -137,8 +137,8 @@ func TestResolver_NewDAG(t *testing.T) { }, }, }, - "testdata/includes/include_a.yaml": { - Includes: []string{"testdata/includes/without_includes.yaml"}, + "testdata/include/common_a.yaml": { + Includes: []string{"testdata/include/no_includes.yaml"}, LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -150,8 +150,8 @@ func TestResolver_NewDAG(t *testing.T) { }, }, }, - "testdata/includes/include_b.yaml": { - Includes: []string{"testdata/includes/without_includes.yaml"}, + "testdata/include/common_b.yaml": { + Includes: []string{"testdata/include/no_includes.yaml"}, LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", @@ -163,7 +163,7 @@ func TestResolver_NewDAG(t *testing.T) { }, }, }, - "testdata/includes/without_includes.yaml": { + "testdata/include/no_includes.yaml": { LavaVersion: ptr("v1.0.0"), ChecktypeURLs: []string{ "checktypes.json", diff --git a/internal/config/dag/dag_test.go b/internal/config/dag/dag_test.go index 29616e1..110e41a 100644 --- a/internal/config/dag/dag_test.go +++ b/internal/config/dag/dag_test.go @@ -15,13 +15,13 @@ func TestDAG_Contains(t *testing.T) { want bool }{ { - name: "Vertex has been added", + name: "existing vertex", vertices: []string{"a", "b", "c"}, vertex: "a", want: true, }, { - name: "Vertex has not been added", + name: "unknown vertex", vertices: []string{"a", "b", "c"}, vertex: "d", want: false, @@ -36,7 +36,7 @@ func TestDAG_Contains(t *testing.T) { } } if got := dag.Contains(tt.vertex); got != tt.want { - t.Errorf("HasVertexBeenIncluded() = %v, want %v", got, tt.want) + t.Errorf("unexpected result: %v", got) } }) } @@ -50,13 +50,13 @@ func TestDAG_AddVertex(t *testing.T) { wantErr error }{ { - name: "add unique vertex", + name: "unique vertex", vertices: []string{"a"}, vertex: "b", wantErr: nil, }, { - name: "add duplicate vertex", + name: "duplicated vertex", vertices: []string{"a"}, vertex: "a", wantErr: ErrDuplicatedVertex, @@ -70,15 +70,8 @@ func TestDAG_AddVertex(t *testing.T) { t.Fatalf("failed to add vertex %s: %v", v, err) } } - _, err := dag.AddVertex(tt.vertex) - if tt.wantErr != nil { - if !errors.Is(err, tt.wantErr) { - t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr) - } - } else { - if err != nil { - t.Errorf("unexpected error adding the vertex: %v", err) - } + if _, err := dag.AddVertex(tt.vertex); !errors.Is(err, tt.wantErr) { + t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr) } }) } @@ -94,7 +87,7 @@ func TestDAG_AddEdge(t *testing.T) { wantErr error }{ { - name: "add edge", + name: "valid", vertices: []string{"a", "b"}, edges: [][]string{}, from: "a", @@ -102,7 +95,7 @@ func TestDAG_AddEdge(t *testing.T) { wantErr: nil, }, { - name: "add edge to invalid vertex", + name: "to invalid vertex", vertices: []string{"a", "b"}, edges: [][]string{}, from: "a", @@ -110,7 +103,7 @@ func TestDAG_AddEdge(t *testing.T) { wantErr: ErrUnknownVertex, }, { - name: "add edge from invalid vertex", + name: "from invalid vertex", vertices: []string{"a", "b"}, edges: [][]string{}, from: "c", @@ -118,7 +111,7 @@ func TestDAG_AddEdge(t *testing.T) { wantErr: ErrUnknownVertex, }, { - name: "add duplicate edge", + name: "duplicated edge", vertices: []string{"a", "b"}, edges: [][]string{{"a", "b"}}, from: "a", @@ -126,7 +119,7 @@ func TestDAG_AddEdge(t *testing.T) { wantErr: ErrDuplicatedEdge, }, { - name: "add a cycle", + name: "cycle", vertices: []string{"a", "b", "c"}, edges: [][]string{{"a", "b"}, {"b", "c"}}, from: "c", @@ -147,15 +140,8 @@ func TestDAG_AddEdge(t *testing.T) { t.Errorf("failed to add edge %s, %s: %v", edge[0], edge[1], err) } } - err := dag.AddEdge(tt.from, tt.to) - if tt.wantErr != nil { - if !errors.Is(err, tt.wantErr) { - t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr) - } - } else { - if err != nil { - t.Errorf("unexpected error adding the edge: %v", err) - } + if err := dag.AddEdge(tt.from, tt.to); !errors.Is(err, tt.wantErr) { + t.Errorf("unexpected error: got: %v, want: %v", err, tt.wantErr) } }) } diff --git a/internal/config/merge_test.go b/internal/config/merge_test.go index cdb9d7f..2501d0d 100644 --- a/internal/config/merge_test.go +++ b/internal/config/merge_test.go @@ -9,7 +9,7 @@ import ( "github.com/google/go-cmp/cmp" ) -func TestLavaMerger_Merge(t *testing.T) { +func TestMerge(t *testing.T) { tests := []struct { name string dst Config @@ -18,14 +18,14 @@ func TestLavaMerger_Merge(t *testing.T) { wantErr bool }{ { - name: "Two empty configurations", + name: "empty configurations", dst: Config{}, src: Config{}, want: Config{}, wantErr: false, }, { - name: "Simple case", + name: "simple", dst: Config{}, src: Config{ LavaVersion: ptr("v1.0.0"), @@ -36,7 +36,7 @@ func TestLavaMerger_Merge(t *testing.T) { wantErr: false, }, { - name: "Settings with default values won't override", + name: "default values do not override", dst: Config{ LavaVersion: ptr("v1.0.0"), AgentConfig: AgentConfig{ @@ -101,7 +101,7 @@ func TestLavaMerger_Merge(t *testing.T) { wantErr: false, }, { - name: "Override value", + name: "override", dst: Config{ LavaVersion: ptr("v1.0.0"), AgentConfig: AgentConfig{ @@ -204,7 +204,7 @@ func TestLavaMerger_Merge(t *testing.T) { wantErr: false, }, { - name: "Append Exclusions", + name: "append exclusions", dst: Config{ ReportConfig: ReportConfig{ Exclusions: []Exclusion{ @@ -238,7 +238,7 @@ func TestLavaMerger_Merge(t *testing.T) { wantErr: false, }, { - name: "Duplicated Exclusions", + name: "duplicated exclusions", dst: Config{ ReportConfig: ReportConfig{ Exclusions: []Exclusion{ diff --git a/internal/config/testdata/includes/autoinclude.yaml b/internal/config/testdata/include/autoinclude.yaml similarity index 74% rename from internal/config/testdata/includes/autoinclude.yaml rename to internal/config/testdata/include/autoinclude.yaml index dda34b9..3b49f94 100644 --- a/internal/config/testdata/includes/autoinclude.yaml +++ b/internal/config/testdata/include/autoinclude.yaml @@ -1,6 +1,6 @@ lava: v1.0.0 include: - - testdata/includes/autoinclude.yaml + - testdata/include/autoinclude.yaml checktypes: - checktypes.json targets: diff --git a/internal/config/testdata/include/common.yaml b/internal/config/testdata/include/common.yaml new file mode 100644 index 0000000..fa0a638 --- /dev/null +++ b/internal/config/testdata/include/common.yaml @@ -0,0 +1,9 @@ +lava: v1.0.0 +include: + - testdata/include/common_a.yaml + - testdata/include/common_b.yaml +checktypes: + - checktypes.json +targets: + - identifier: example.com + type: DomainName diff --git a/internal/config/testdata/includes/include_a.yaml b/internal/config/testdata/include/common_a.yaml similarity index 71% rename from internal/config/testdata/includes/include_a.yaml rename to internal/config/testdata/include/common_a.yaml index aff5336..af6d477 100644 --- a/internal/config/testdata/includes/include_a.yaml +++ b/internal/config/testdata/include/common_a.yaml @@ -1,6 +1,6 @@ lava: v1.0.0 include: - - testdata/includes/without_includes.yaml + - testdata/include/no_includes.yaml checktypes: - checktypes.json targets: diff --git a/internal/config/testdata/includes/include_b.yaml b/internal/config/testdata/include/common_b.yaml similarity index 71% rename from internal/config/testdata/includes/include_b.yaml rename to internal/config/testdata/include/common_b.yaml index aff5336..af6d477 100644 --- a/internal/config/testdata/includes/include_b.yaml +++ b/internal/config/testdata/include/common_b.yaml @@ -1,6 +1,6 @@ lava: v1.0.0 include: - - testdata/includes/without_includes.yaml + - testdata/include/no_includes.yaml checktypes: - checktypes.json targets: diff --git a/internal/config/testdata/include/duplicated.yaml b/internal/config/testdata/include/duplicated.yaml new file mode 100644 index 0000000..437be37 --- /dev/null +++ b/internal/config/testdata/include/duplicated.yaml @@ -0,0 +1,9 @@ +lava: v1.0.0 +include: + - testdata/include/no_includes.yaml + - testdata/include/no_includes.yaml +checktypes: + - checktypes.json +targets: + - identifier: example.com + type: DomainName diff --git a/internal/config/testdata/includes/include_local.yaml b/internal/config/testdata/include/local.yaml similarity index 71% rename from internal/config/testdata/includes/include_local.yaml rename to internal/config/testdata/include/local.yaml index aff5336..af6d477 100644 --- a/internal/config/testdata/includes/include_local.yaml +++ b/internal/config/testdata/include/local.yaml @@ -1,6 +1,6 @@ lava: v1.0.0 include: - - testdata/includes/without_includes.yaml + - testdata/include/no_includes.yaml checktypes: - checktypes.json targets: diff --git a/internal/config/testdata/includes/without_includes.yaml b/internal/config/testdata/include/no_includes.yaml similarity index 100% rename from internal/config/testdata/includes/without_includes.yaml rename to internal/config/testdata/include/no_includes.yaml diff --git a/internal/config/testdata/includes/include_duplicated.yaml b/internal/config/testdata/includes/include_duplicated.yaml deleted file mode 100644 index d63cfbf..0000000 --- a/internal/config/testdata/includes/include_duplicated.yaml +++ /dev/null @@ -1,9 +0,0 @@ -lava: v1.0.0 -include: - - testdata/includes/without_includes.yaml - - testdata/includes/without_includes.yaml -checktypes: - - checktypes.json -targets: - - identifier: example.com - type: DomainName diff --git a/internal/config/testdata/includes/include_two_with_common_include.yaml b/internal/config/testdata/includes/include_two_with_common_include.yaml deleted file mode 100644 index f5ffd58..0000000 --- a/internal/config/testdata/includes/include_two_with_common_include.yaml +++ /dev/null @@ -1,9 +0,0 @@ -lava: v1.0.0 -include: - - testdata/includes/include_a.yaml - - testdata/includes/include_b.yaml -checktypes: - - checktypes.json -targets: - - identifier: example.com - type: DomainName