From a4d90c3ac9e86a4099d445e731a235fdce793180 Mon Sep 17 00:00:00 2001 From: Nathan Wallace Date: Sat, 15 Nov 2025 21:10:53 +0800 Subject: [PATCH] Fix logic error in IdentifyMissingComments closes #4814 (#4872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add test for #4814: Logic error in IdentifyMissingComments Demonstrates the bug where the function uses OR (||) instead of AND (&&) on line 426, causing connections being deleted to be incorrectly added to MissingComments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Fix #4814: Change OR to AND in IdentifyMissingComments logic Corrects the logic error on line 426 where OR (||) was incorrectly used instead of AND (&&). The condition should be "if NOT updating AND NOT deleting" to properly exclude connections being updated or deleted from the MissingComments list. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- pkg/steampipeconfig/connection_updates.go | 2 +- .../connection_updates_test.go | 138 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 pkg/steampipeconfig/connection_updates_test.go diff --git a/pkg/steampipeconfig/connection_updates.go b/pkg/steampipeconfig/connection_updates.go index 309ff4159..600c10362 100644 --- a/pkg/steampipeconfig/connection_updates.go +++ b/pkg/steampipeconfig/connection_updates.go @@ -423,7 +423,7 @@ func (u *ConnectionUpdates) IdentifyMissingComments() { if !currentState.CommentsSet { _, updating := u.Update[name] _, deleting := u.Delete[name] - if !updating || deleting { + if !updating && !deleting { log.Printf("[TRACE] connection %s comments not set, marking as missing", name) u.MissingComments[name] = state } diff --git a/pkg/steampipeconfig/connection_updates_test.go b/pkg/steampipeconfig/connection_updates_test.go new file mode 100644 index 000000000..99f4dc465 --- /dev/null +++ b/pkg/steampipeconfig/connection_updates_test.go @@ -0,0 +1,138 @@ +package steampipeconfig + +import ( + "testing" + + "github.com/turbot/steampipe/v2/pkg/constants" +) + +// TestConnectionUpdates_IdentifyMissingComments tests the logic error in IdentifyMissingComments +// Bug #4814: The function uses OR (||) when it should use AND (&&) on line 426 +// Current buggy logic: if !updating || deleting +// This means connections being DELETED are still added to MissingComments +// Expected logic: if !updating && !deleting +func TestConnectionUpdates_IdentifyMissingComments(t *testing.T) { + tests := []struct { + name string + connectionName string + currentState *ConnectionState + finalState *ConnectionState + isUpdating bool + isDeleting bool + shouldBeMissing bool + description string + }{ + { + name: "connection being deleted should NOT be in MissingComments", + connectionName: "conn1", + currentState: &ConnectionState{ + ConnectionName: "conn1", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + CommentsSet: false, // Comments not set + }, + finalState: &ConnectionState{ + ConnectionName: "conn1", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + }, + isUpdating: false, + isDeleting: true, // Being deleted + shouldBeMissing: false, // Should NOT be in MissingComments (but bug adds it) + description: "Deleting connections should be ignored", + }, + { + name: "connection being updated should NOT be in MissingComments", + connectionName: "conn2", + currentState: &ConnectionState{ + ConnectionName: "conn2", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + CommentsSet: false, + }, + finalState: &ConnectionState{ + ConnectionName: "conn2", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + }, + isUpdating: true, // Being updated + isDeleting: false, + shouldBeMissing: false, // Should NOT be in MissingComments + description: "Updating connections should be ignored", + }, + { + name: "stable connection without comments SHOULD be in MissingComments", + connectionName: "conn3", + currentState: &ConnectionState{ + ConnectionName: "conn3", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + CommentsSet: false, // Comments not set + }, + finalState: &ConnectionState{ + ConnectionName: "conn3", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + }, + isUpdating: false, // Not being updated + isDeleting: false, // Not being deleted + shouldBeMissing: true, // SHOULD be in MissingComments + description: "Stable connections without comments should be identified", + }, + { + name: "connection with comments set should NOT be in MissingComments", + connectionName: "conn4", + currentState: &ConnectionState{ + ConnectionName: "conn4", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + CommentsSet: true, // Comments ARE set + }, + finalState: &ConnectionState{ + ConnectionName: "conn4", + Plugin: "test_plugin", + State: constants.ConnectionStateReady, + }, + isUpdating: false, + isDeleting: false, + shouldBeMissing: false, // Should NOT be in MissingComments + description: "Connections with comments set should be ignored", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create ConnectionUpdates with the test scenario + updates := &ConnectionUpdates{ + Update: make(ConnectionStateMap), + Delete: make(map[string]struct{}), + MissingComments: make(ConnectionStateMap), + CurrentConnectionState: make(ConnectionStateMap), + FinalConnectionState: make(ConnectionStateMap), + } + + // Set up current and final state + updates.CurrentConnectionState[tt.connectionName] = tt.currentState + updates.FinalConnectionState[tt.connectionName] = tt.finalState + + // Set up updating/deleting status + if tt.isUpdating { + updates.Update[tt.connectionName] = tt.finalState + } + if tt.isDeleting { + updates.Delete[tt.connectionName] = struct{}{} + } + + // Call the function under test + updates.IdentifyMissingComments() + + // Check if the connection is in MissingComments + _, inMissingComments := updates.MissingComments[tt.connectionName] + + if tt.shouldBeMissing != inMissingComments { + t.Errorf("%s: expected shouldBeMissing=%v, got inMissingComments=%v", + tt.description, tt.shouldBeMissing, inMissingComments) + } + }) + } +}