Skip to content
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

chore: Creates project for test execution #2010

Merged
merged 6 commits into from
Mar 13, 2024
Merged

chore: Creates project for test execution #2010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d515f8b
execution project
lantoli Mar 12, 2024
68030d6
no need to empty these values
lantoli Mar 12, 2024
1d0fe09
simplify TestMainExecution conditions
lantoli Mar 13, 2024
c29b2ce
address feedback
lantoli Mar 13, 2024
1a1f21c
add comments
lantoli Mar 13, 2024
d6bb0d6
clarify ProjectIDExecution behavior
lantoli Mar 13, 2024
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
2 changes: 1 addition & 1 deletion ...nearchive/data_source_privatelink_endpoint_service_data_federation_online_archive_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var (

func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchiveDS_basic(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
Copy link
Member Author

Choose a reason for hiding this comment

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

all functions like this will start with ProjectID, we have ProjectIDExecution, ProjectIDGlobal and might have ProjectIDTrigger, etc.

endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
customerEndpointDNSName = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_DNS_NAME")
)
Expand Down
2 changes: 1 addition & 1 deletion ...earchive/data_source_privatelink_endpoint_service_data_federation_online_archives_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var (

func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchivesDSPlural_basic(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
)

Expand Down
2 changes: 1 addition & 1 deletion ...ve/resource_privatelink_endpoint_service_data_federation_online_archive_migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

func TestAccMigrationNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_basic(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
config = resourceConfigBasic(projectID, endpointID, comment)
)
Expand Down
13 changes: 10 additions & 3 deletions ...nlinearchive/resource_privatelink_endpoint_service_data_federation_online_archive_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@ var (
atlasRegion = "US_EAST_1"
)

func TestMain(m *testing.M) {
Copy link
Member Author

@lantoli lantoli Mar 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Go runs this function before any test in the package

acc.SetupSharedResources()
exitCode := m.Run()
acc.CleanupSharedResources()
os.Exit(exitCode)
}

func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_basic(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
)

Expand Down Expand Up @@ -51,7 +58,7 @@ func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_basic(t
}
func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_updateComment(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
commentUpdated = "Terraform Acceptance Test Updated"
)
Expand Down Expand Up @@ -98,7 +105,7 @@ func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_updateC

func TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_basicWithRegionDnsName(t *testing.T) {
var (
projectID = acc.ProjectIDGlobal(t)
projectID = acc.ProjectIDExecution(t)
endpointID = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_ID")
customerEndpointDNSName = os.Getenv("MONGODB_ATLAS_PRIVATE_ENDPOINT_DNS_NAME")
)
Expand Down
39 changes: 39 additions & 0 deletions internal/testutil/acc/atlas.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package acc

import (
"context"
"fmt"
"os"
"testing"

"github.com/stretchr/testify/require"
"go.mongodb.org/atlas-sdk/v20231115007/admin"
)

func createProject(tb testing.TB, name string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] this looks like a more generic util method to be put outside this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

moved

tb.Helper()
orgID := os.Getenv("MONGODB_ATLAS_ORG_ID")
require.NotNil(tb, "Project creation failed: %s, org not set", name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
require.NotNil(tb, "Project creation failed: %s, org not set", name)
require.NotNilf(tb, "Project creation failed: %s, org not set", name)

params := &admin.Group{Name: name, OrgId: orgID}
resp, _, err := ConnV2().ProjectsApi.CreateProject(context.Background(), params).Execute()
require.NoError(tb, err, "Project creation failed: %s, err: %s", name, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
require.NoError(tb, err, "Project creation failed: %s, err: %s", name, err)
require.NoErrorf(tb, err, "Project creation failed: %s, err: %s", name, err)

id := resp.GetId()
require.NotEmpty(tb, id, "Project creation failed: %s", name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
require.NotEmpty(tb, id, "Project creation failed: %s", name)
require.NotEmptyf(tb, id, "Project creation failed: %s", name)

return id
}

func deleteProject(id string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity why create is a helper but delete is not?

Copy link
Member Author

Choose a reason for hiding this comment

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

the creation is done lazily inside a test so we have the T, but deletion is done in TestMain when all test have finished and we don't have test context anymore

_, _, err := ConnV2().ProjectsApi.DeleteProject(context.Background(), id).Execute()
if err != nil {
fmt.Printf("Project deletion failed: %s, error: %s", id, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

which is delete is a helper you can then do

Suggested change
fmt.Printf("Project deletion failed: %s, error: %s", id, err)
tb.Logf("Project deletion failed: %s, error: %s", id, err)

Copy link
Member Author

Choose a reason for hiding this comment

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

can't be because it's called from TestMain not from a test

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, this may make this approach a bit noisy on the logs since you usually only want logs when running on verbose mode (-v) and right now with printf is harder to control

Copy link
Member Author

Choose a reason for hiding this comment

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

we're running with -v in GH Actions, but you have a point, will consider in next PRs if delete it, thanks!

}
}

func projectID(tb testing.TB, name string) string {
tb.Helper()
SkipInUnitTest(tb)
resp, _, _ := ConnV2().ProjectsApi.GetProjectByName(context.Background(), name).Execute()
id := resp.GetId()
require.NotEmpty(tb, id, "Project name not found: %s", name)
Copy link
Collaborator

@gssbzn gssbzn Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

can you make sure all assertions where you do string formatting end with f for format, other wise is a concatenation

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they do the same, e.g.:

a := "hello"
	require.Empty(t, a, "a is not empty: %s, watchout", a)

Error:      	Should be empty, but was hello
        	Test:       	TestAccNetworkPrivatelinkEndpointServiceDataFederationOnlineArchive_basic
        	Messages:   	a is not empty: hello, watchout

the signature is: func Empty(t TestingT, object interface{}, msgAndArgs ...interface{}) {

example of implementation:

//	assert.NotNilf(t, err, "error message %s", "formatted")
func NotNilf(t TestingT, object interface{}, msg string, args ...interface{}) bool {
	if h, ok := t.(tHelper); ok {
		h.Helper()
	}
	return NotNil(t, object, append([]interface{}{msg}, args...)...)
}

Copy link
Member Author

Choose a reason for hiding this comment

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

i think it's just to make sure that the first param is a string (with the msg)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I see internally does fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) I assumed this was closer to t.Error vs t.Errorf 🤷

return id
}
11 changes: 0 additions & 11 deletions internal/testutil/acc/name.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package acc

import (
"context"
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/helper/acctest"
"github.com/stretchr/testify/require"
)

const (
Expand Down Expand Up @@ -45,12 +43,3 @@ func ProjectIDGlobal(tb testing.TB) string {
tb.Helper()
return projectID(tb, prefixProjectKeep+"-global")
}

func projectID(tb testing.TB, name string) string {
tb.Helper()
SkipInUnitTest(tb)
resp, _, _ := ConnV2().ProjectsApi.GetProjectByName(context.Background(), name).Execute()
id := resp.GetId()
require.NotEmpty(tb, id, "Project name not found: %s", name)
return id
}
49 changes: 49 additions & 0 deletions internal/testutil/acc/shared_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package acc

import (
"fmt"
"sync"
"testing"

"github.com/stretchr/testify/require"
)

// SetupSharedResources must be called from TestMain test package in order to use ProjectIDExecution.
func SetupSharedResources() {
sharedInfo.init = true
}

// CleanupSharedResources must be called from TestMain test package in order to use ProjectIDExecution.
func CleanupSharedResources() {
if sharedInfo.projectID != "" {
fmt.Printf("Deleting execution project: %s, id: %s\n", sharedInfo.projectName, sharedInfo.projectID)
Copy link
Collaborator

Choose a reason for hiding this comment

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

how are we running tests? normally you want logs when running in verbose mode -v

Copy link
Member Author

Choose a reason for hiding this comment

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

we use verbose flag:

TF_ACC=1 go test $(ACCTEST_PACKAGES) -run '$(ACCTEST_REGEX_RUN)' -skip '$(ACCTEST_REGEX_SKIP)' -v -parallel $(PARALLEL_GO_TEST) $(TESTARGS) -timeout $(ACCTEST_TIMEOUT) -ldflags="$(LINKER_FLAGS)"

deleteProject(sharedInfo.projectID)
}
}

// ProjectIDExecution returns a project id created for the execution of the tests in the resource package.
// Even if a GH test group is run, every resource/package will create its own project, not a shared project for all the test group.
func ProjectIDExecution(tb testing.TB) string {
tb.Helper()
SkipInUnitTest(tb)
require.True(tb, sharedInfo.init, "SetupSharedResources must called from TestMain test package")

sharedInfo.mu.Lock()
defer sharedInfo.mu.Unlock()

// lazy creation so it's only done if really needed
if sharedInfo.projectID == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe support os.Getenv("MONGODB_ATLAS_PROJECT_ID") similar to acc.GetClusterInfo?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that is a good idea, but i'm going to do in a next PR so this doesn't get more complicated, thx!

sharedInfo.projectName = RandomProjectName()
tb.Logf("Creating execution project: %s\n", sharedInfo.projectName)
sharedInfo.projectID = createProject(tb, sharedInfo.projectName)
}

return sharedInfo.projectID
}

var sharedInfo = struct {
projectID string
projectName string
mu sync.Mutex
init bool
}{}
6 changes: 5 additions & 1 deletion internal/testutil/acc/skip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ func SkipTestForCI(tb testing.TB) {
// This can be useful for acceptance tests that define logic prior to resource.Test/resource.ParallelTest functions as this code would always be run.
func SkipInUnitTest(tb testing.TB) {
tb.Helper()
if os.Getenv("TF_ACC") == "" {
if InUnitTest() {
tb.Skip()
}
}

func InUnitTest() bool {
return os.Getenv("TF_ACC") == ""
}