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.

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: Add test time measurements #3411

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/Snowflake-Labs/terraform-provider-snowflake

go 1.22.10
go 1.24.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're upgrading to 1.23 in another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I know; this should be reversed to the one used in dev. I had to bump to see if t.Context() would be helpful.


require (
github.com/DATA-DOG/go-sqlmock v1.5.2
Expand Down
74 changes: 74 additions & 0 deletions pkg/internal/measurement/test_measurements.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package measurement

import (
"fmt"
"golang.org/x/exp/maps"

Check failure on line 5 in pkg/internal/measurement/test_measurements.go

View workflow job for this annotation

GitHub Actions / reviewdog

[golangci] reported by reviewdog 🐶 File is not properly formatted (gofumpt) Raw Output: pkg/internal/measurement/test_measurements.go:5:1: File is not properly formatted (gofumpt) "golang.org/x/exp/maps" ^
"slices"
"strings"
"testing"
"time"

Check failure on line 9 in pkg/internal/measurement/test_measurements.go

View workflow job for this annotation

GitHub Actions / reviewdog

[golangci] reported by reviewdog 🐶 File is not properly formatted (gofumpt) Raw Output: pkg/internal/measurement/test_measurements.go:9:1: File is not properly formatted (gofumpt) "time" ^
)

type TestMeasurement struct {
TestName string
Duration time.Duration
SubMeasurements map[string]*TestMeasurement
}

func NewTestMeasurement(t *testing.T) *TestMeasurement {

Check failure on line 18 in pkg/internal/measurement/test_measurements.go

View workflow job for this annotation

GitHub Actions / reviewdog

[golangci] reported by reviewdog 🐶 test helper function should start from t.Helper() (thelper) Raw Output: pkg/internal/measurement/test_measurements.go:18:6: test helper function should start from t.Helper() (thelper) func NewTestMeasurement(t *testing.T) *TestMeasurement { ^
m := new(TestMeasurement)
m.TestName = t.Name()
m.SubMeasurements = make(map[string]*TestMeasurement)
return m
}

type TestMeasurements struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if we really need this. Can we just pass []TestMeasurement to the functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe it could be like that, I'll take a look next time.

Measurements map[string]*TestMeasurement
}

func NewTestMeasurements() *TestMeasurements {
return &TestMeasurements{
Measurements: make(map[string]*TestMeasurement),
}
}

func PrintMeasurementSummary(measurements *TestMeasurements, truncateDuration time.Duration) {
fmt.Println("Summary of test measurements (sorted by execution time)")
values := maps.Values(measurements.Measurements)
slices.SortFunc(values, func(a, b *TestMeasurement) int {
return int(b.Duration.Nanoseconds() - a.Duration.Nanoseconds())
})
printMeasurements(0, values, truncateDuration)
}

func printMeasurements(level int, measurements []*TestMeasurement, truncateDuration time.Duration) {
for _, measurement := range measurements {
fmt.Printf("%-10s%s%s\n", measurement.Duration.Truncate(truncateDuration), strings.Repeat(" → ", level), measurement.TestName)
printMeasurements(level+1, maps.Values(measurement.SubMeasurements), truncateDuration)
}
}

func MeasureTestTime(testMeasurements *TestMeasurements, t *testing.T) {

Check failure on line 51 in pkg/internal/measurement/test_measurements.go

View workflow job for this annotation

GitHub Actions / reviewdog

[golangci] reported by reviewdog 🐶 parameter *testing.T should be the first or after context.Context (thelper) Raw Output: pkg/internal/measurement/test_measurements.go:51:6: parameter *testing.T should be the first or after context.Context (thelper) func MeasureTestTime(testMeasurements *TestMeasurements, t *testing.T) { ^
t.Helper()
start := time.Now()
m := NewTestMeasurement(t)
testNameParts := strings.Split(m.TestName, "/")
if len(testNameParts) == 1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: IMO this whole block could be extracted and simplified as a recursion. Also, why we're not doing a map check with panic in else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It could be done with recursion, but this also works. I'll see next why I didn't check for panic (or value presence in the map).

if _, ok := testMeasurements.Measurements[m.TestName]; ok {
panic("this shouldn't happen")
}

testMeasurements.Measurements[m.TestName] = m
} else {
currentMeasurement := testMeasurements.Measurements[testNameParts[0]]
for index, part := range testNameParts[1:] {
if value, ok := currentMeasurement.SubMeasurements[part]; ok {
currentMeasurement = value
} else {
m.TestName = strings.Join(testNameParts[index+1:], "/")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not clear, why we're omitting the previous parts and joining the next ones?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's only for visualization purposes, instead of seeing

Test
-> Test/Sub-Test

you'll see

Test
-> Sub-Test

currentMeasurement.SubMeasurements[part] = m
}
}
}
t.Cleanup(func() { m.Duration = time.Now().Sub(start) })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls add a method in TestMeasurement for this.

}
68 changes: 68 additions & 0 deletions pkg/sdk/accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,64 @@ import (
"github.com/stretchr/testify/require"
)

func TestATest(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about moving to pkg/internal/measurement/

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll either move or remove it completely. It was used only for testing purposes.

measureTest(t)
// TODO: TestTest not found; add it to the map

t.Run("level one", func(t *testing.T) {
measureTest(t)
// TODO: TestTest/level_one (TestTest found; add level_one as sub-measurement)

t.Run("level two", func(t *testing.T) {
measureTest(t)
// TODO: TestTest/level_one/level_two (TestTest found;

t.Run("level three", func(t *testing.T) {
measureTest(t)
})
})

t.Run("2nd level two", func(t *testing.T) {
measureTest(t)

t.Run("2nd level three", func(t *testing.T) {
measureTest(t)
})
})
})

t.Run("2nd level one", func(t *testing.T) {
measureTest(t)
// TODO: TestTest/level_one (TestTest found; add level_one as sub-measurement)

t.Run("level two", func(t *testing.T) {
measureTest(t)
// TODO: TestTest/level_one/level_two (TestTest found;

t.Run("level three", func(t *testing.T) {
measureTest(t)

t.Run("level four", func(t *testing.T) {
measureTest(t)

t.Run("level five", func(t *testing.T) {
measureTest(t)
})

t.Run("2nd level five", func(t *testing.T) {
measureTest(t)
})
})
})
})
})
}

func TestAccountCreate(t *testing.T) {
measureTest(t)

t.Run("simplest case", func(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()
password := random.Password()
opts := &CreateAccountOptions{
Expand All @@ -25,6 +81,7 @@ func TestAccountCreate(t *testing.T) {
})

t.Run("every option", func(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()
key := random.Password()
opts := &CreateAccountOptions{
Expand All @@ -46,6 +103,7 @@ func TestAccountCreate(t *testing.T) {
})

t.Run("static password", func(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()
password := random.Password()
opts := &CreateAccountOptions{
Expand All @@ -66,6 +124,8 @@ func TestAccountCreate(t *testing.T) {
}

func TestAccountAlter(t *testing.T) {
measureTest(t)

t.Run("validation: exactly one value set in AccountSet - nothing set", func(t *testing.T) {
opts := &AlterAccountOptions{
Set: &AccountSet{},
Expand Down Expand Up @@ -357,6 +417,8 @@ func TestAccountAlter(t *testing.T) {
}

func TestAccountDrop(t *testing.T) {
measureTest(t)

t.Run("validate: empty options", func(t *testing.T) {
opts := &DropAccountOptions{}
assertOptsInvalidJoinedErrors(t, opts, ErrInvalidObjectIdentifier)
Expand All @@ -383,6 +445,8 @@ func TestAccountDrop(t *testing.T) {
}

func TestAccountShow(t *testing.T) {
measureTest(t)

t.Run("empty options", func(t *testing.T) {
opts := &ShowAccountOptions{}
assertOptsValidAndSQLEquals(t, opts, `SHOW ACCOUNTS`)
Expand All @@ -409,6 +473,8 @@ func TestAccountShow(t *testing.T) {
}

func TestToAccountCreateResponse(t *testing.T) {
measureTest(t)

testCases := []struct {
Name string
RawInput string
Expand Down Expand Up @@ -522,6 +588,8 @@ func TestToAccountCreateResponse(t *testing.T) {
}

func TestToAccountEdition(t *testing.T) {
measureTest(t)

type test struct {
input string
want AccountEdition
Expand Down
5 changes: 5 additions & 0 deletions pkg/sdk/alerts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
)

func TestAlertCreate(t *testing.T) {
measureTest(t)
id := randomSchemaObjectIdentifier()

t.Run("with complete options", func(t *testing.T) {
Expand All @@ -31,6 +32,7 @@ func TestAlertCreate(t *testing.T) {
}

func TestAlertAlter(t *testing.T) {
measureTest(t)
id := randomSchemaObjectIdentifier()

t.Run("fail without alter action specified", func(t *testing.T) {
Expand Down Expand Up @@ -115,6 +117,7 @@ func TestAlertAlter(t *testing.T) {
}

func TestAlertDrop(t *testing.T) {
measureTest(t)
id := randomSchemaObjectIdentifier()

t.Run("empty options", func(t *testing.T) {
Expand All @@ -139,6 +142,7 @@ func TestAlertDrop(t *testing.T) {
}

func TestAlertShow(t *testing.T) {
measureTest(t)
id := randomSchemaObjectIdentifier()

t.Run("empty options", func(t *testing.T) {
Expand Down Expand Up @@ -212,6 +216,7 @@ func TestAlertShow(t *testing.T) {
}

func TestAlertDescribe(t *testing.T) {
measureTest(t)
id := randomSchemaObjectIdentifier()

t.Run("empty options", func(t *testing.T) {
Expand Down
5 changes: 5 additions & 0 deletions pkg/sdk/api_integrations_gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const (
)

func TestApiIntegrations_Create(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

// Minimal valid CreateApiIntegrationOptions for AWS
Expand Down Expand Up @@ -123,6 +124,7 @@ func TestApiIntegrations_Create(t *testing.T) {
}

func TestApiIntegrations_Alter(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

// Minimal valid AlterApiIntegrationOptions
Expand Down Expand Up @@ -303,6 +305,7 @@ func TestApiIntegrations_Alter(t *testing.T) {
}

func TestApiIntegrations_Drop(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

// Minimal valid DropApiIntegrationOptions
Expand Down Expand Up @@ -331,6 +334,7 @@ func TestApiIntegrations_Drop(t *testing.T) {
}

func TestApiIntegrations_Show(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

// Minimal valid ShowApiIntegrationOptions
Expand Down Expand Up @@ -358,6 +362,7 @@ func TestApiIntegrations_Show(t *testing.T) {
}

func TestApiIntegrations_Describe(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

// Minimal valid DescribeApiIntegrationOptions
Expand Down
4 changes: 4 additions & 0 deletions pkg/sdk/application_packages_gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sdk
import "testing"

func TestApplicationPackages_Create(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *CreateApplicationPackageOptions {
Expand Down Expand Up @@ -42,6 +43,7 @@ func TestApplicationPackages_Create(t *testing.T) {
}

func TestApplicationPackages_Alter(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *AlterApplicationPackageOptions {
Expand Down Expand Up @@ -210,6 +212,7 @@ func TestApplicationPackages_Alter(t *testing.T) {
}

func TestApplicationPackages_Drop(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *DropApplicationPackageOptions {
Expand Down Expand Up @@ -237,6 +240,7 @@ func TestApplicationPackages_Drop(t *testing.T) {
}

func TestApplicationPackages_Show(t *testing.T) {
measureTest(t)
defaultOpts := func() *ShowApplicationPackageOptions {
return &ShowApplicationPackageOptions{}
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/sdk/application_roles_gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sdk
import "testing"

func TestApplicationRoles_Grant(t *testing.T) {
measureTest(t)
id := randomDatabaseObjectIdentifier()

// Minimal valid GrantApplicationRoleOptions
Expand Down Expand Up @@ -59,6 +60,7 @@ func TestApplicationRoles_Grant(t *testing.T) {
}

func TestApplicationRoles_Revoke(t *testing.T) {
measureTest(t)
id := randomDatabaseObjectIdentifier()

// Minimal valid RevokeApplicationRoleOptions
Expand Down Expand Up @@ -110,6 +112,7 @@ func TestApplicationRoles_Revoke(t *testing.T) {
}

func TestApplicationRoles_Show(t *testing.T) {
measureTest(t)
appId := randomAccountObjectIdentifier()

// Minimal valid ShowApplicationRoleOptions
Expand Down
5 changes: 5 additions & 0 deletions pkg/sdk/applications_gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
)

func TestApplications_Create(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()
pid := randomAccountObjectIdentifier()

Expand Down Expand Up @@ -91,6 +92,7 @@ func TestApplications_Create(t *testing.T) {
}

func TestApplications_Alter(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *AlterApplicationOptions {
Expand Down Expand Up @@ -228,6 +230,7 @@ func TestApplications_Alter(t *testing.T) {
}

func TestApplications_Drop(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *DropApplicationOptions {
Expand Down Expand Up @@ -255,6 +258,7 @@ func TestApplications_Drop(t *testing.T) {
}

func TestApplications_Describe(t *testing.T) {
measureTest(t)
id := randomAccountObjectIdentifier()

defaultOpts := func() *DescribeApplicationOptions {
Expand All @@ -281,6 +285,7 @@ func TestApplications_Describe(t *testing.T) {
}

func TestApplications_Show(t *testing.T) {
measureTest(t)
defaultOpts := func() *ShowApplicationOptions {
return &ShowApplicationOptions{}
}
Expand Down
Loading
Loading