From 9422766e17cf955d11792e999c0cbd5cf85f2939 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:34:20 +0100 Subject: [PATCH] chore: remove some integration test flakiness (#8818) Remove some integration test flakiness. --------- Co-authored-by: Livio Spring --- ...ons_allow_public_org_registrations_test.go | 45 +++++++------ .../restrictions_allowed_languages_test.go | 67 ++++++++++--------- .../admin/integration_test/server_test.go | 18 ----- .../integration_test/execution_target_test.go | 9 ++- .../v3alpha/integration_test/query_test.go | 2 +- internal/integration/assert.go | 2 +- 6 files changed, 68 insertions(+), 75 deletions(-) diff --git a/internal/api/grpc/admin/integration_test/restrictions_allow_public_org_registrations_test.go b/internal/api/grpc/admin/integration_test/restrictions_allow_public_org_registrations_test.go index 3663fec4dd..3cbcf8abd0 100644 --- a/internal/api/grpc/admin/integration_test/restrictions_allow_public_org_registrations_test.go +++ b/internal/api/grpc/admin/integration_test/restrictions_allow_public_org_registrations_test.go @@ -10,6 +10,7 @@ import ( "net/http/cookiejar" "net/url" "testing" + "time" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" @@ -70,28 +71,34 @@ func awaitPubOrgRegDisallowed(t *testing.T, ctx context.Context, cc *integration // awaitGetSSRGetResponse cuts the CSRF token from the response body if it exists func awaitGetSSRGetResponse(t *testing.T, ctx context.Context, client *http.Client, parsedURL *url.URL, expectCode int) string { var csrfToken []byte - await(t, ctx, func(tt *assert.CollectT) { - resp, err := client.Get(parsedURL.String()) - require.NoError(tt, err) - body, err := io.ReadAll(resp.Body) - require.NoError(tt, err) - searchField := ``)) - } - assert.Equal(tt, resp.StatusCode, expectCode) - }) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, + func(tt *assert.CollectT) { + resp, err := client.Get(parsedURL.String()) + require.NoError(tt, err) + body, err := io.ReadAll(resp.Body) + require.NoError(tt, err) + searchField := ``)) + } + assert.Equal(tt, resp.StatusCode, expectCode) + }, retryDuration, tick, "awaiting successful get SSR get response failed", + ) return string(csrfToken) } // awaitPostFormResponse needs a valid CSRF token to make it to the actual endpoint implementation and get the expected status code func awaitPostFormResponse(t *testing.T, ctx context.Context, client *http.Client, parsedURL *url.URL, expectCode int, csrfToken string) { - await(t, ctx, func(tt *assert.CollectT) { - resp, err := client.PostForm(parsedURL.String(), url.Values{ - "gorilla.csrf.Token": {csrfToken}, - }) - require.NoError(tt, err) - assert.Equal(tt, resp.StatusCode, expectCode) - }) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, + func(tt *assert.CollectT) { + resp, err := client.PostForm(parsedURL.String(), url.Values{ + "gorilla.csrf.Token": {csrfToken}, + }) + require.NoError(tt, err) + assert.Equal(tt, resp.StatusCode, expectCode) + }, retryDuration, tick, "awaiting successful Post Form failed", + ) } diff --git a/internal/api/grpc/admin/integration_test/restrictions_allowed_languages_test.go b/internal/api/grpc/admin/integration_test/restrictions_allowed_languages_test.go index 92f688a900..e00b7f221b 100644 --- a/internal/api/grpc/admin/integration_test/restrictions_allowed_languages_test.go +++ b/internal/api/grpc/admin/integration_test/restrictions_allowed_languages_test.go @@ -51,7 +51,7 @@ func TestServer_Restrictions_AllowedLanguages(t *testing.T) { require.Equal(ttt, language.Make(defaultLang.Language), language.English) }) tt.Run("the discovery endpoint returns all supported languages", func(ttt *testing.T) { - awaitDiscoveryEndpoint(ttt, instance.Domain, supportedLanguagesStr, nil) + awaitDiscoveryEndpoint(ttt, ctx, instance.Domain, supportedLanguagesStr, nil) }) }) t.Run("restricting the default language fails", func(tt *testing.T) { @@ -92,10 +92,10 @@ func TestServer_Restrictions_AllowedLanguages(t *testing.T) { require.Condition(tt, contains(supported.GetLanguages(), supportedLanguagesStr)) }) t.Run("the disallowed language is not listed in the discovery endpoint", func(tt *testing.T) { - awaitDiscoveryEndpoint(tt, instance.Domain, []string{defaultAndAllowedLanguage.String()}, []string{disallowedLanguage.String()}) + awaitDiscoveryEndpoint(tt, ctx, instance.Domain, []string{defaultAndAllowedLanguage.String()}, []string{disallowedLanguage.String()}) }) t.Run("the login ui is rendered in the default language", func(tt *testing.T) { - awaitLoginUILanguage(tt, instance.Domain, disallowedLanguage, defaultAndAllowedLanguage, "Passwort") + awaitLoginUILanguage(tt, ctx, instance.Domain, disallowedLanguage, defaultAndAllowedLanguage, "Passwort") }) t.Run("preferred languages are not restricted by the supported languages", func(tt *testing.T) { tt.Run("change user profile", func(ttt *testing.T) { @@ -153,10 +153,10 @@ func TestServer_Restrictions_AllowedLanguages(t *testing.T) { t.Run("allowing the language makes it usable again", func(tt *testing.T) { tt.Run("the previously disallowed language is listed in the discovery endpoint again", func(ttt *testing.T) { - awaitDiscoveryEndpoint(ttt, instance.Domain, []string{disallowedLanguage.String()}, nil) + awaitDiscoveryEndpoint(ttt, ctx, instance.Domain, []string{disallowedLanguage.String()}, nil) }) tt.Run("the login ui is rendered in the previously disallowed language", func(ttt *testing.T) { - awaitLoginUILanguage(ttt, instance.Domain, disallowedLanguage, disallowedLanguage, "Contraseña") + awaitLoginUILanguage(ttt, ctx, instance.Domain, disallowedLanguage, disallowedLanguage, "Contraseña") }) }) } @@ -164,36 +164,36 @@ func TestServer_Restrictions_AllowedLanguages(t *testing.T) { func setAndAwaitAllowedLanguages(ctx context.Context, cc *integration.Client, t *testing.T, selectLanguages []string) { _, err := cc.Admin.SetRestrictions(ctx, &admin.SetRestrictionsRequest{AllowedLanguages: &admin.SelectLanguages{List: selectLanguages}}) require.NoError(t, err) - awaitCtx, awaitCancel := context.WithTimeout(ctx, 10*time.Second) - defer awaitCancel() - await(t, awaitCtx, func(tt *assert.CollectT) { - restrictions, getErr := cc.Admin.GetRestrictions(awaitCtx, &admin.GetRestrictionsRequest{}) - expectLanguages := selectLanguages - if len(selectLanguages) == 0 { - expectLanguages = nil - } - assert.NoError(tt, getErr) - assert.Equal(tt, expectLanguages, restrictions.GetAllowedLanguages()) - }) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, + func(tt *assert.CollectT) { + restrictions, getErr := cc.Admin.GetRestrictions(ctx, &admin.GetRestrictionsRequest{}) + expectLanguages := selectLanguages + if len(selectLanguages) == 0 { + expectLanguages = nil + } + assert.NoError(tt, getErr) + assert.Equal(tt, expectLanguages, restrictions.GetAllowedLanguages()) + }, retryDuration, tick, "awaiting successful GetAllowedLanguages failed", + ) } func setAndAwaitDefaultLanguage(ctx context.Context, cc *integration.Client, t *testing.T, lang language.Tag) { _, err := cc.Admin.SetDefaultLanguage(ctx, &admin.SetDefaultLanguageRequest{Language: lang.String()}) require.NoError(t, err) - awaitCtx, awaitCancel := context.WithTimeout(ctx, 10*time.Second) - defer awaitCancel() - await(t, awaitCtx, func(tt *assert.CollectT) { - defaultLang, getErr := cc.Admin.GetDefaultLanguage(awaitCtx, &admin.GetDefaultLanguageRequest{}) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + defaultLang, getErr := cc.Admin.GetDefaultLanguage(ctx, &admin.GetDefaultLanguageRequest{}) assert.NoError(tt, getErr) assert.Equal(tt, lang.String(), defaultLang.GetLanguage()) - }) + }, retryDuration, tick, "awaiting successful GetDefaultLanguage failed", + ) } -func awaitDiscoveryEndpoint(t *testing.T, domain string, containsUILocales, notContainsUILocales []string) { - awaitCtx, awaitCancel := context.WithTimeout(context.Background(), 10*time.Second) - defer awaitCancel() - await(t, awaitCtx, func(tt *assert.CollectT) { - req, err := http.NewRequestWithContext(awaitCtx, http.MethodGet, "http://"+domain+":8080/.well-known/openid-configuration", nil) +func awaitDiscoveryEndpoint(t *testing.T, ctx context.Context, domain string, containsUILocales, notContainsUILocales []string) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+domain+":8080/.well-known/openid-configuration", nil) require.NoError(tt, err) resp, err := http.DefaultClient.Do(req) require.NoError(tt, err) @@ -213,14 +213,14 @@ func awaitDiscoveryEndpoint(t *testing.T, domain string, containsUILocales, notC if notContainsUILocales != nil { assert.Condition(tt, not(contains(doc.UILocalesSupported, notContainsUILocales))) } - }) + }, retryDuration, tick, "awaiting successful call to Discovery endpoint failed", + ) } -func awaitLoginUILanguage(t *testing.T, domain string, acceptLanguage language.Tag, expectLang language.Tag, containsText string) { - awaitCtx, awaitCancel := context.WithTimeout(context.Background(), 10*time.Second) - defer awaitCancel() - await(t, awaitCtx, func(tt *assert.CollectT) { - req, err := http.NewRequestWithContext(awaitCtx, http.MethodGet, "http://"+domain+":8080/ui/login/register", nil) +func awaitLoginUILanguage(t *testing.T, ctx context.Context, domain string, acceptLanguage language.Tag, expectLang language.Tag, containsText string) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, "http://"+domain+":8080/ui/login/register", nil) req.Header.Set("Accept-Language", acceptLanguage.String()) require.NoError(tt, err) resp, err := http.DefaultClient.Do(req) @@ -232,7 +232,8 @@ func awaitLoginUILanguage(t *testing.T, domain string, acceptLanguage language.T }() require.NoError(tt, err) assert.Containsf(tt, string(body), containsText, "login ui language is in "+expectLang.String()) - }) + }, retryDuration, tick, "awaiting successful LoginUI in specific language failed", + ) } // We would love to use assert.Contains here, but it doesn't work with slices of strings diff --git a/internal/api/grpc/admin/integration_test/server_test.go b/internal/api/grpc/admin/integration_test/server_test.go index e29b4d5c78..d762b08bfb 100644 --- a/internal/api/grpc/admin/integration_test/server_test.go +++ b/internal/api/grpc/admin/integration_test/server_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/zitadel/zitadel/internal/integration" admin_pb "github.com/zitadel/zitadel/pkg/grpc/admin" @@ -34,23 +33,6 @@ func TestMain(m *testing.M) { }()) } -func await(t *testing.T, ctx context.Context, cb func(*assert.CollectT)) { - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) - require.EventuallyWithT( - t, - func(tt *assert.CollectT) { - defer func() { - // Panics are not recovered and don't mark the test as failed, so we need to do that ourselves - assert.Nil(tt, recover(), "panic in await callback") - }() - cb(tt) - }, - retryDuration, - tick, - "awaiting successful callback failed", - ) -} - var _ assert.TestingT = (*noopAssertionT)(nil) type noopAssertionT struct{} diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go index b62d0ee37f..c70ed227c1 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go @@ -288,7 +288,9 @@ func waitForExecutionOnCondition(ctx context.Context, t *testing.T, instance *in if !assert.NoError(ttt, err) { return } - assert.Len(ttt, got.GetResult(), 1) + if !assert.Len(ttt, got.GetResult(), 1) { + return + } gotTargets := got.GetResult()[0].GetExecution().GetTargets() // always first check length, otherwise its failed anyway if assert.Len(ttt, gotTargets, len(targets)) { @@ -296,7 +298,6 @@ func waitForExecutionOnCondition(ctx context.Context, t *testing.T, instance *in assert.EqualExportedValues(ttt, targets[i].GetType(), gotTargets[i].GetType()) } } - }, retryDuration, tick, "timeout waiting for expected execution result") return } @@ -316,7 +317,9 @@ func waitForTarget(ctx context.Context, t *testing.T, instance *integration.Inst if !assert.NoError(ttt, err) { return } - assert.Len(ttt, got.GetResult(), 1) + if !assert.Len(ttt, got.GetResult(), 1) { + return + } config := got.GetResult()[0].GetConfig() assert.Equal(ttt, config.GetEndpoint(), endpoint) switch ty { diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go index e3d3233604..d6f584b35e 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go @@ -216,7 +216,7 @@ func TestServer_GetTarget(t *testing.T) { err := tt.args.dep(tt.args.ctx, tt.args.req, tt.want) require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 2*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.ActionV3Alpha.GetTarget(tt.args.ctx, tt.args.req) if tt.wantErr { diff --git a/internal/integration/assert.go b/internal/integration/assert.go index 8e875ee48e..6743c8297e 100644 --- a/internal/integration/assert.go +++ b/internal/integration/assert.go @@ -107,7 +107,7 @@ func AssertListDetails[L ListDetails, D ListDetailsMsg[L]](t assert.TestingT, ex if wantDetails.GetTimestamp() != nil { gotCD := gotDetails.GetTimestamp().AsTime() wantCD := time.Now() - assert.WithinRange(t, gotCD, wantCD.Add(-1*time.Minute), wantCD.Add(time.Minute)) + assert.WithinRange(t, gotCD, wantCD.Add(-10*time.Minute), wantCD.Add(time.Minute)) } }