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

feat: make router-tests less flaky #1484

Open
wants to merge 47 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
3e55410
fix: wrong increment in "subscribe ws with filter"
alepane21 Jan 3, 2025
e1454c5
chore: remove retry integration tests
alepane21 Jan 3, 2025
6bb7887
fix: stricter condition on test
alepane21 Jan 3, 2025
f05a5dd
fix: force max timeout on nats shutdown
alepane21 Jan 4, 2025
d03a4fc
fix: less strict condition on test
alepane21 Jan 7, 2025
1727feb
Merge branch 'refs/heads/main' into ale/eng-6219-make-router-tests-le…
alepane21 Jan 7, 2025
e91500e
fix: less strict condition on test
alepane21 Jan 7, 2025
f3ce9c9
fix: stricter condition on test
alepane21 Jan 7, 2025
51ec7b8
fix: stricter condition on test
alepane21 Jan 7, 2025
c1f79de
chore: change a require to an assert to see next assertion value
alepane21 Jan 8, 2025
70f34c8
chore: try a different order of the checks for subscribe sync
alepane21 Jan 8, 2025
42ca681
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 8, 2025
82f7408
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 8, 2025
eeda567
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 8, 2025
a0d380a
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 10, 2025
7af651e
fix: use fixed graphql-go-tools
alepane21 Jan 13, 2025
14f2622
fix: use specified metric reader
alepane21 Jan 13, 2025
b17e17c
chore: limit parallelization
alepane21 Jan 13, 2025
4158be9
chore: changed flaky test to give more details when an error occurs
alepane21 Jan 15, 2025
e3f5d59
fix: always call the env Shutdown
alepane21 Jan 15, 2025
440f2e1
fix: static check
alepane21 Jan 15, 2025
d794103
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 15, 2025
478b768
fix: set higher timeout default on events checks
alepane21 Jan 15, 2025
bab0852
fix: add WaitForTriggerCount
alepane21 Jan 15, 2025
4b3499e
Merge branch 'refs/heads/main' into ale/eng-6219-make-router-tests-le…
alepane21 Jan 20, 2025
3101301
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 21, 2025
f2c6d40
feat: now the nats provider can log errors
alepane21 Jan 21, 2025
6f05f79
feat: try less time sensitive approach with multipart tests
alepane21 Jan 21, 2025
233207a
chore: wait before publishing second message
alepane21 Jan 21, 2025
907043c
fix: deflake ws connections
alepane21 Jan 22, 2025
3accd1c
fix: deflake subscribe sse with filter
alepane21 Jan 22, 2025
985522a
fix: make retry waiting time grow each iteration
alepane21 Jan 22, 2025
51c47b2
fix: make deflake functions return the error
alepane21 Jan 22, 2025
8e8353e
fix: deflake "subscribe sync sse works without query param"
alepane21 Jan 27, 2025
a220066
Merge branch 'refs/heads/main' into ale/eng-6219-make-router-tests-le…
alepane21 Jan 27, 2025
c726296
fix: add Shutdown to cleanup methods also on error
alepane21 Jan 27, 2025
a7c4426
fix: unflake two hot reload test
alepane21 Jan 28, 2025
6d3b17c
chore: try to execute only flaky test with retry
alepane21 Jan 28, 2025
afdae6a
chore: more readable regexp
alepane21 Jan 28, 2025
6af87ed
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 28, 2025
6e7714b
chore: make flaky test less parallel, to see if the behave better
alepane21 Jan 28, 2025
d68a4a2
chore: try different deflake params
alepane21 Jan 29, 2025
79103b8
chore: retry flaky tests
alepane21 Jan 29, 2025
34b46fd
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 29, 2025
af74896
chore: use updated graphql-go-tools
alepane21 Jan 29, 2025
7d81d64
fix: waiting URL
alepane21 Jan 29, 2025
6dfc928
Merge branch 'main' into ale/eng-6219-make-router-tests-less-flaky
alepane21 Jan 29, 2025
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
8 changes: 6 additions & 2 deletions .github/workflows/router-ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,19 @@ jobs:
with:
cache-dependency-path: |
router-tests/go.sum
- uses: nick-fields/retry@v3
- name: Run Integration tests
working-directory: ./router-tests
run: make test test_params="-run '^Test[^(Flaky)]' --timeout=5m --parallel 10"
- name: Run Flaky Integration tests
uses: nick-fields/retry@v3
with:
timeout_minutes: 30
max_attempts: 5
retry_wait_seconds: 5
retry_on: error
command: |
cd router-tests
make test test_params="--timeout=5m"
make test test_params="-run '^TestFlaky' --timeout=5m -p 1 --parallel 1"

image_scan:
if: github.event.pull_request.head.repo.full_name == github.repository
Expand Down
2 changes: 1 addition & 1 deletion demo/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/wundergraph/cosmo/composition-go v0.0.0-20240124120900-5effe48a4a1d
github.com/wundergraph/cosmo/router v0.0.0-20250119174948-4b991294658e
github.com/wundergraph/cosmo/router-tests v0.0.0-20241213115435-a249dba8c52a
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.145
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.146
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.46.1
go.opentelemetry.io/otel v1.28.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.23.1
Expand Down
4 changes: 2 additions & 2 deletions demo/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ github.com/wundergraph/cosmo/router v0.0.0-20250119174948-4b991294658e h1:ee4fu7
github.com/wundergraph/cosmo/router v0.0.0-20250119174948-4b991294658e/go.mod h1:ImqCvxvvNOy1UxbuTnFtin/CDBFHoFqrZly3rC2z+e0=
github.com/wundergraph/cosmo/router-tests v0.0.0-20241213115435-a249dba8c52a h1:GVLe85f5g+G0IOorDBBNTfm5Ua9DO0vuVY7ReSTOEbQ=
github.com/wundergraph/cosmo/router-tests v0.0.0-20241213115435-a249dba8c52a/go.mod h1:I+SFviFnd3BHlPmYn+ckmzQyDB9+/c8RZJo4t6VQAds=
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.145 h1:3JuBmRux6YB/UZgh6COvgLXzQhMIsdHV7A02NsYdAVE=
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.145/go.mod h1:B7eV0Qh8Lop9QzIOQcsvKp3S0ejfC6mgyWoJnI917yQ=
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.146 h1:C9+jjMgbU/RJTiFGC0HNHan4LxrY7fIhmbZRoqZryLk=
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.146/go.mod h1:B7eV0Qh8Lop9QzIOQcsvKp3S0ejfC6mgyWoJnI917yQ=
github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 h1:gEOO8jv9F4OT7lGCjxCBTO/36wtF6j2nSip77qHd4x4=
github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
Expand Down
3 changes: 2 additions & 1 deletion router-tests/cache_warmup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,8 @@ func TestCacheWarmup(t *testing.T) {
})
}

func TestCacheWarmupMetrics(t *testing.T) {
// Is set as Flaky so that when running the tests it will be run separately and retried if it fails
func TestFlakyCacheWarmupMetrics(t *testing.T) {
t.Run("should emit planning times metrics during warmup", func(t *testing.T) {
t.Parallel()

Expand Down
7 changes: 7 additions & 0 deletions router-tests/complexity_limits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import (
"net/http"
"testing"
"time"

"github.com/stretchr/testify/require"
"github.com/wundergraph/cosmo/router-tests/testenv"
Expand Down Expand Up @@ -152,6 +153,8 @@ func TestComplexityLimits(t *testing.T) {
require.Contains(t, testSpan.Attributes(), otel.WgQueryDepth.Int(3))
require.Contains(t, testSpan.Attributes(), otel.WgQueryDepthCacheHit.Bool(false))
exporter.Reset()
// wait to let cache get consistent
time.Sleep(100 * time.Millisecond)

failedRes2, _ := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Query: `{ employee(id:1) { id details { forename surname } } }`,
Expand All @@ -163,6 +166,8 @@ func TestComplexityLimits(t *testing.T) {
require.Contains(t, testSpan2.Attributes(), otel.WgQueryDepth.Int(3))
require.Contains(t, testSpan2.Attributes(), otel.WgQueryDepthCacheHit.Bool(true))
exporter.Reset()
// wait to let cache get consistent
time.Sleep(100 * time.Millisecond)

successRes := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
Query: `query { employees { id } }`,
Expand All @@ -172,6 +177,8 @@ func TestComplexityLimits(t *testing.T) {
require.Contains(t, testSpan3.Attributes(), otel.WgQueryDepth.Int(2))
require.Contains(t, testSpan3.Attributes(), otel.WgQueryDepthCacheHit.Bool(false))
exporter.Reset()
// wait to let cache get consistent
time.Sleep(100 * time.Millisecond)

successRes2 := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
Query: `query { employees { id } }`,
Expand Down
34 changes: 22 additions & 12 deletions router-tests/config_hot_reload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/wundergraph/cosmo/router/pkg/routerconfig"

"github.com/gorilla/websocket"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/wundergraph/cosmo/router-tests/testenv"
"github.com/wundergraph/cosmo/router/core"
Expand Down Expand Up @@ -238,28 +239,35 @@ func TestConfigHotReload(t *testing.T) {
},
}, func(t *testing.T, xEnv *testenv.Environment) {

var done atomic.Bool

var startedReq atomic.Bool
go func() {
defer done.Store(true)

startedReq.Store(true)
res, err := xEnv.MakeGraphQLRequestWithContext(context.Background(), testenv.GraphQLRequest{
Query: `{ employees { id } }`,
})
require.NoError(t, err)
require.Equal(t, res.Response.StatusCode, 200)
require.Equal(t, `{"errors":[{"message":"Failed to fetch from Subgraph 'employees'."}],"data":{"employees":null}}`, res.Body)
assert.Equal(t, res.Response.StatusCode, 200)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use assert, rather than require here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I know, you should use the require only if you don't want the next assertion to run. In this instance I wanted to see the next assertion result to help me debug the issue when the test fails.

assert.Equal(t, `{"errors":[{"message":"Failed to fetch from Subgraph 'employees'."}],"data":{"employees":null}}`, res.Body)
}()

// Let's wait a bit to make sure all requests are in flight
// otherwise the shutdown will be too fast and the wait-group will not be done fully
require.Eventually(t, func() bool {
return startedReq.Load()
}, time.Second*10, time.Millisecond*100)
time.Sleep(time.Millisecond * 100)

xEnv.Shutdown()
var done atomic.Bool
go func() {
defer done.Store(true)

err := xEnv.Router.Shutdown(context.Background())
assert.ErrorContains(t, err, context.DeadlineExceeded.Error())
}()

require.Eventually(t, func() bool {
return done.Load()
}, time.Second*5, time.Millisecond*100)
}, time.Second*20, time.Millisecond*100)
})
})

Expand Down Expand Up @@ -314,13 +322,15 @@ func TestConfigHotReload(t *testing.T) {

// Swap config
require.NoError(t, pm.updateConfig(pm.initConfig, "old-1"))

err = conn.ReadJSON(&msg)

// Ensure that the connection is closed. In the future, we might want to send a complete message to the client
// If the operation happen fast enough, ensure that the connection is closed.
// In the future, we might want to send a complete message to the client
// and wait until in-flight messages are delivered before closing the connection
var wsErr *websocket.CloseError
require.ErrorAs(t, err, &wsErr)
if err != nil {
var wsErr *websocket.CloseError
require.ErrorAs(t, err, &wsErr)
}

require.NoError(t, conn.Close())

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package integration_test
package events_test

import (
"github.com/stretchr/testify/assert"
Expand Down
Loading