-
-
Notifications
You must be signed in to change notification settings - Fork 959
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
test: deflake and parallelize persister tests #3953
Conversation
func WithConfigValues(ctx context.Context, setValues map[string]any) context.Context { | ||
values, ok := ctx.Value(contextConfigKey).([]map[string]any) | ||
if !ok { | ||
values = make(mapProvider) | ||
} | ||
expandedValues := make([]map[string]any, 0, len(newValues)) | ||
for k, v := range newValues { | ||
parts := strings.Split(k, ".") | ||
val := map[string]any{parts[len(parts)-1]: v} | ||
if len(parts) > 1 { | ||
for i := len(parts) - 2; i >= 0; i-- { | ||
val = map[string]any{parts[i]: val} | ||
} | ||
} | ||
expandedValues = append(expandedValues, val) | ||
} | ||
for _, v := range expandedValues { | ||
maps.Merge(v, values) | ||
values = make([]map[string]any, 0) | ||
} | ||
newValues := make([]map[string]any, len(values), len(values)+1) | ||
copy(newValues, values) | ||
newValues = append(newValues, setValues) | ||
|
||
return context.WithValue(ctx, contextConfigKey, values) | ||
return context.WithValue(ctx, contextConfigKey, newValues) |
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.
Kinda copied from your branch @aeneasr, but adjusted slightly to be a slice of maps. The benefit of that is that overrides still work in the correct order deterministically:
ctx = WithConfigValues(ctx, map[string]any{"serve": map[string]any{"public": x}})
ctx = WithConfigValues(ctx, map[string]any{"serve.public": x})
The keys in this case would not be the same, so both would be part of the map after merge, but only one of the values would be used at random.
defer otelx.End(span, &err) | ||
|
||
var ec errorx.ErrorContainer | ||
if err := p.GetConnection(ctx).Where("id = ? AND nid = ?", id, p.NetworkID(ctx)).First(&ec); err != nil { | ||
return nil, sqlcon.HandleError(err) | ||
} | ||
|
||
if err := p.GetConnection(ctx).RawQuery( | ||
"UPDATE selfservice_errors SET was_seen = true, seen_at = ? WHERE id = ? AND nid = ?", | ||
time.Now().UTC(), id, p.NetworkID(ctx)).Exec(); err != nil { | ||
return nil, sqlcon.HandleError(err) | ||
if err := p.Transaction(ctx, func(ctx context.Context, c *pop.Connection) error { | ||
if err := c.Where("id = ? AND nid = ?", id, p.NetworkID(ctx)).First(&ec); err != nil { | ||
return sqlcon.HandleError(err) | ||
} | ||
|
||
if err := c.RawQuery( | ||
"UPDATE selfservice_errors SET was_seen = true, seen_at = ? WHERE id = ? AND nid = ?", |
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.
Ideally we could use a RETURNING
clause here, but obviously mysql does not support that...
defer span.End() | ||
h := hmac.New(sha512.New512_256, secret) | ||
_, _ = h.Write([]byte(value)) | ||
return fmt.Sprintf("%x", h.Sum(nil)) | ||
} | ||
|
||
func (p *Persister) hmacConstantCompare(ctx context.Context, value, hash string) bool { |
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.
Unused function.
@@ -94,7 +94,7 @@ func pl(t testing.TB) func(lvl logging.Level, s string, args ...interface{}) { | |||
|
|||
func createCleanDatabases(t testing.TB) map[string]*driver.RegistryDefault { | |||
conns := map[string]string{ | |||
"sqlite": "sqlite://file:" + t.TempDir() + "/db.sqlite?_fk=true", | |||
"sqlite": "sqlite://file:" + t.TempDir() + "/db.sqlite?_fk=true&max_conns=1&lock=false", |
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.
This allows to run parallel tests with sqlite, because a connection pool of 1 will never execute parallel queries 🤯
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3953 +/- ##
==========================================
- Coverage 78.17% 78.16% -0.01%
==========================================
Files 363 363
Lines 25453 25443 -10
==========================================
- Hits 19898 19888 -10
Misses 4032 4032
Partials 1523 1523 ☔ View full report in Codecov by Sentry. |
Moving more tests to the context-based test configs, and enabling parallelism everywhere 🎉