Skip to content

Commit 1ef5555

Browse files
Better Handle Empty Tables (#1228)
* utils improvements and new tests * [autofix.ci] apply automated fixes * lint improvements * clean code --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent ffb6c08 commit 1ef5555

File tree

4 files changed

+188
-22
lines changed

4 files changed

+188
-22
lines changed

cmd/list_metadata.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
listerrors "github.com/cloudposse/atmos/pkg/list/errors"
1616
fl "github.com/cloudposse/atmos/pkg/list/flags"
1717
f "github.com/cloudposse/atmos/pkg/list/format"
18-
u "github.com/cloudposse/atmos/pkg/list/utils"
1918
"github.com/cloudposse/atmos/pkg/schema"
2019
utils "github.com/cloudposse/atmos/pkg/utils"
2120
)
@@ -160,10 +159,5 @@ func listMetadata(cmd *cobra.Command, args []string) (string, error) {
160159
return "", err
161160
}
162161

163-
if output == "" || u.IsEmptyTable(output) {
164-
logNoMetadataFoundMessage(params.ComponentFilter)
165-
return "", nil
166-
}
167-
168162
return output, nil
169163
}

cmd/list_settings.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
listerrors "github.com/cloudposse/atmos/pkg/list/errors"
1616
fl "github.com/cloudposse/atmos/pkg/list/flags"
1717
f "github.com/cloudposse/atmos/pkg/list/format"
18-
u "github.com/cloudposse/atmos/pkg/list/utils"
1918
"github.com/cloudposse/atmos/pkg/schema"
2019
utils "github.com/cloudposse/atmos/pkg/utils"
2120
)
@@ -164,10 +163,5 @@ func listSettings(cmd *cobra.Command, args []string) (string, error) {
164163
return "", err
165164
}
166165

167-
if output == "" || u.IsEmptyTable(output) {
168-
logNoSettingsFoundMessage(params.ComponentFilter)
169-
return "", nil
170-
}
171-
172166
return output, nil
173167
}

pkg/list/utils/utils.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,6 @@ func IsNoValuesFoundError(err error) bool {
1414
return ok
1515
}
1616

17-
// IsEmptyTable checks if the output is an empty table (only contains headers and formatting).
18-
func IsEmptyTable(output string) bool {
19-
if output == "" {
20-
return true
21-
}
22-
23-
newlineCount := strings.Count(output, "\n")
24-
return newlineCount <= 4
25-
}
26-
2717
// CheckComponentExists checks if a component exists in the Atmos configuration.
2818
// It returns true if the component exists, false otherwise.
2919
func CheckComponentExists(atmosConfig *schema.AtmosConfiguration, componentName string) bool {

pkg/list/utils/utils_test.go

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
package utils_test
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"testing"
7+
8+
listErrors "github.com/cloudposse/atmos/pkg/list/errors"
9+
"github.com/cloudposse/atmos/pkg/list/utils"
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
// Mock NoValuesFoundError for testing IsNoValuesFoundError.
14+
type mockNoValuesFoundError struct {
15+
message string
16+
}
17+
18+
func (e *mockNoValuesFoundError) Error() string {
19+
return e.message
20+
}
21+
22+
func newMockNoValuesFoundError(message string) error {
23+
return &mockNoValuesFoundError{message: message}
24+
}
25+
26+
func TestIsNoValuesFoundError(t *testing.T) {
27+
tests := []struct {
28+
name string
29+
err error
30+
expected bool
31+
}{
32+
{
33+
name: "Test with custom mock error resembling NoValuesFoundError",
34+
err: newMockNoValuesFoundError("No values found"),
35+
expected: false, // Mock type doesn't match *errors.NoValuesFoundError
36+
},
37+
{
38+
name: "Test with standard error",
39+
err: fmt.Errorf("A standard error"),
40+
expected: false,
41+
},
42+
{
43+
name: "Test with nil error",
44+
err: nil,
45+
expected: false,
46+
},
47+
{
48+
name: "Test with actual NoValuesFoundError",
49+
// Instantiate directly as a struct pointer
50+
err: &listErrors.NoValuesFoundError{},
51+
expected: true,
52+
},
53+
}
54+
55+
for _, tt := range tests {
56+
t.Run(tt.name, func(t *testing.T) {
57+
actualOk := utils.IsNoValuesFoundError(tt.err)
58+
assert.Equal(t, tt.expected, actualOk)
59+
})
60+
}
61+
}
62+
63+
// TestCheckComponentExists focuses on the logic *within* CheckComponentExists,
64+
// specifically the processing of the map structure returned by ExecuteDescribeStacks.
65+
// It does not test the ExecuteDescribeStacks call itself or its error handling.
66+
func TestCheckComponentExistsLogic(t *testing.T) {
67+
// Simulate the map structure that ExecuteDescribeStacks would return
68+
simulatedStacksMap := map[string]interface{}{
69+
"dev/stack1": map[string]interface{}{ // Valid structure
70+
"components": map[string]interface{}{
71+
"terraform": map[string]interface{}{
72+
"comp-a": map[string]interface{}{"var": "val1"},
73+
"comp-b": map[string]interface{}{"var": "val2"},
74+
},
75+
"helmfile": map[string]interface{}{ // Should be ignored
76+
"comp-c": map[string]interface{}{"var": "val3"},
77+
},
78+
},
79+
},
80+
"prod/stack2": map[string]interface{}{ // Valid structure
81+
"components": map[string]interface{}{
82+
"terraform": map[string]interface{}{
83+
"comp-a": map[string]interface{}{"var": "val_prod"}, // Duplicate comp, still exists
84+
"comp-d": map[string]interface{}{"var": "val4"},
85+
},
86+
},
87+
},
88+
"staging/stack3": map[string]interface{}{ // Malformed: components is not a map
89+
"components": "this is not a map",
90+
},
91+
"test/stack4": map[string]interface{}{ // Malformed: terraform is not a map
92+
"components": map[string]interface{}{
93+
"terraform": "this is not a map",
94+
},
95+
},
96+
"edge/stack5": "this is not a map", // Malformed: stackData itself is not a map
97+
"empty/stack6": map[string]interface{}{ // Malformed: components exists but is empty map
98+
"components": map[string]interface{}{},
99+
},
100+
"empty/stack7": map[string]interface{}{ // Malformed: terraform exists but is empty map
101+
"components": map[string]interface{}{
102+
"terraform": map[string]interface{}{},
103+
},
104+
},
105+
}
106+
107+
// Helper function to mimic the core processing loop of CheckComponentExists
108+
processMapForComponent := func(stacksMap map[string]interface{}, componentName string) bool {
109+
if componentName == "" { // Check the initial guard from the original function
110+
return false
111+
}
112+
parts := strings.Split(componentName, "/")
113+
baseName := parts[len(parts)-1]
114+
115+
for _, stackData := range stacksMap {
116+
stackMap, ok := stackData.(map[string]interface{})
117+
if !ok { // Covers line 47
118+
continue
119+
}
120+
121+
componentsMap, ok := stackMap["components"].(map[string]interface{})
122+
if !ok { // Covers line 51
123+
continue
124+
}
125+
126+
terraformComponents, ok := componentsMap["terraform"].(map[string]interface{})
127+
if !ok { // Covers line 57
128+
continue
129+
}
130+
131+
_, exists := terraformComponents[baseName]
132+
if exists { // Covers line 63 (true path)
133+
return true
134+
}
135+
}
136+
return false // Covers line 68 (component not found after checking all stacks)
137+
}
138+
139+
tests := []struct {
140+
name string
141+
componentName string
142+
expected bool
143+
}{
144+
{
145+
name: "Test component exists (comp-a)",
146+
componentName: "comp-a",
147+
expected: true,
148+
},
149+
{
150+
name: "Test component exists (comp-b)",
151+
componentName: "comp-b",
152+
expected: true,
153+
},
154+
{
155+
name: "Test component exists (comp-d)",
156+
componentName: "comp-d",
157+
expected: true,
158+
},
159+
{
160+
name: "Test component exists with path (infra/comp-a)",
161+
componentName: "infra/comp-a", // Should extract 'comp-a'
162+
expected: true,
163+
},
164+
{
165+
name: "Test component does not exist (comp-x)",
166+
componentName: "comp-x",
167+
expected: false,
168+
},
169+
{
170+
name: "Test component only in helmfile (comp-c)",
171+
componentName: "comp-c", // Should not be found in terraform section
172+
expected: false,
173+
},
174+
// Test the initial guard clause separately using the helper
175+
{
176+
name: "Test empty component name (via helper)",
177+
componentName: "",
178+
expected: false,
179+
},
180+
}
181+
182+
for _, tt := range tests {
183+
t.Run(tt.name, func(t *testing.T) {
184+
actual := processMapForComponent(simulatedStacksMap, tt.componentName)
185+
assert.Equal(t, tt.expected, actual)
186+
})
187+
}
188+
}

0 commit comments

Comments
 (0)