Skip to content

Improve tests in internal/states (ProviderAddrs, HasCurrent, Unlock) #37650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 2, 2025
Merged

Improve tests in internal/states (ProviderAddrs, HasCurrent, Unlock) #37650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e123492
Improve tests in internal/states (ProviderAddrs, HasCurrent, Unlock)
davidspielmann Sep 20, 2025
5ca4a43
nil-receiver test for deposeCurrentObject (no panic)
davidspielmann Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions internal/states/resource_test.go
View file
Open in desktop
Copy link
Member

@dsa0x dsa0x Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution and investigation @davidspielmann .

In the test below, it seems that the source of the panic is in the test itself

if got := is.Current; got != nil {
	t.Errorf("current is %#v; want nil", got)
}

(is is nil, hence the panic)

I do agree that we should test that case of where the ResourceInstance is nil. However, it would be better if we are testing our method expectations directly

Something like

t.Run("nil resource", func(t *testing.T) {
		var nilRI *ResourceInstance
		dk = nilRI.deposeCurrentObject(NotDeposed)
		t.Logf("deposedKey is %q", dk)

		if dk != NotDeposed {
			t.Errorf("expected NotDeposed, got %q", dk)
		}
	})

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks a lot for the reply.

I completely agree, your suggested solution is even better!

Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ import (
)

func TestResourceInstanceDeposeCurrentObject(t *testing.T) {
t.Run("nil resource", func(t *testing.T) {
var nilRI *ResourceInstance
dk := nilRI.deposeCurrentObject(NotDeposed)
t.Logf("deposedKey (nil receiver) is %q", dk)

if dk != NotDeposed {
t.Fatalf("expected NotDeposed for nil receiver, got %q", dk)
}
})
obj := &ResourceInstanceObjectSrc{
// Empty for the sake of this test, because we're just going to
// compare by pointer below anyway.
Expand Down
77 changes: 77 additions & 0 deletions internal/states/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"reflect"
"testing"
"time"

"github.com/go-test/deep"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -337,6 +338,18 @@ func TestStateHasResourceInstanceObjects(t *testing.T) {
s := ss.Lock()
delete(s.Modules[""].Resources["test.foo"].Instances, addrs.NoKey)
ss.Unlock()
done := make(chan struct{})
go func() {
ss.Lock()
ss.Unlock()
close(done)
}()
select {
case <-done:
// OK: lock was released
case <-time.After(500 * time.Millisecond):
t.Fatalf("Unlock did not release SyncState lock (timed out acquiring lock again)")
}
},
false,
},
Expand Down Expand Up @@ -1009,6 +1022,70 @@ func TestState_MoveModule(t *testing.T) {
})
}

func TestState_ProviderAddrs(t *testing.T) {
// 1) nil state
var nilState *State
if got := nilState.ProviderAddrs(); got != nil {
t.Fatalf("nil state: expected nil, got %#v", got)
}

// 2) empty state
empty := NewState()
if got := empty.ProviderAddrs(); got != nil {
t.Fatalf("empty state: expected nil, got %#v", got)
}

// 3) populated state
s := NewState()

rootAWS := addrs.AbsProviderConfig{
Module: addrs.RootModule,
Provider: addrs.NewDefaultProvider("aws"),
}
rootGoogle := addrs.AbsProviderConfig{
Module: addrs.RootModule,
Provider: addrs.NewDefaultProvider("google"),
}
childAWS := addrs.AbsProviderConfig{
Module: addrs.RootModule.Child("child"),
Provider: addrs.NewDefaultProvider("aws"),
}

rm := s.RootModule()
rm.SetResourceInstanceCurrent(
addrs.Resource{Mode: addrs.ManagedResourceMode, Type: "test_thing", Name: "foo"}.Instance(addrs.NoKey),
&ResourceInstanceObjectSrc{Status: ObjectReady, SchemaVersion: 1, AttrsJSON: []byte(`{}`)},
rootAWS,
)

rm.SetResourceInstanceCurrent(
addrs.Resource{Mode: addrs.ManagedResourceMode, Type: "test_thing", Name: "bar"}.Instance(addrs.NoKey),
&ResourceInstanceObjectSrc{Status: ObjectReady, SchemaVersion: 1, AttrsJSON: []byte(`{}`)},
rootAWS,
)

rm.SetResourceInstanceCurrent(
addrs.Resource{Mode: addrs.ManagedResourceMode, Type: "test_thing", Name: "baz"}.Instance(addrs.NoKey),
&ResourceInstanceObjectSrc{Status: ObjectReady, SchemaVersion: 1, AttrsJSON: []byte(`{}`)},
rootGoogle,
)

childMI := addrs.RootModuleInstance.Child("child", addrs.NoKey)
cm := s.EnsureModule(childMI)
cm.SetResourceInstanceCurrent(
addrs.Resource{Mode: addrs.ManagedResourceMode, Type: "test_thing", Name: "child"}.Instance(addrs.NoKey),
&ResourceInstanceObjectSrc{Status: ObjectReady, SchemaVersion: 1, AttrsJSON: []byte(`{}`)},
childAWS,
)

got := s.ProviderAddrs()
expected := []addrs.AbsProviderConfig{childAWS, rootAWS, rootGoogle}

if !reflect.DeepEqual(got, expected) {
t.Fatalf("unexpected provider addrs\nexpected: %#v\ngot: %#v", expected, got)
}
}

func mustParseModuleInstanceStr(str string) addrs.ModuleInstance {
addr, diags := addrs.ParseModuleInstanceStr(str)
if diags.HasErrors() {
Expand Down
Loading