-
Notifications
You must be signed in to change notification settings - Fork 430
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
base: dev
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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
|
||
"slices" | ||
"strings" | ||
"testing" | ||
"time" | ||
Check failure on line 9 in pkg/internal/measurement/test_measurements.go
|
||
) | ||
|
||
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
|
||
m := new(TestMeasurement) | ||
m.TestName = t.Name() | ||
m.SubMeasurements = make(map[string]*TestMeasurement) | ||
return m | ||
} | ||
|
||
type TestMeasurements struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we really need this. Can we just pass There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
t.Helper() | ||
start := time.Now() | ||
m := NewTestMeasurement(t) | ||
testNameParts := strings.Split(m.TestName, "/") | ||
if len(testNameParts) == 1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:], "/") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's only for visualization purposes, instead of seeing
you'll see
|
||
currentMeasurement.SubMeasurements[part] = m | ||
} | ||
} | ||
} | ||
t.Cleanup(func() { m.Duration = time.Now().Sub(start) }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pls add a method in |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,64 @@ import ( | |
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestATest(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about moving to There was a problem hiding this comment. Choose a reason for hiding this commentThe 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{ | ||
|
@@ -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{ | ||
|
@@ -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{ | ||
|
@@ -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{}, | ||
|
@@ -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) | ||
|
@@ -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`) | ||
|
@@ -409,6 +473,8 @@ func TestAccountShow(t *testing.T) { | |
} | ||
|
||
func TestToAccountCreateResponse(t *testing.T) { | ||
measureTest(t) | ||
|
||
testCases := []struct { | ||
Name string | ||
RawInput string | ||
|
@@ -522,6 +588,8 @@ func TestToAccountCreateResponse(t *testing.T) { | |
} | ||
|
||
func TestToAccountEdition(t *testing.T) { | ||
measureTest(t) | ||
|
||
type test struct { | ||
input string | ||
want AccountEdition | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ift.Context()
would be helpful.