Fix flaky TestBasicExampleQuery by handling context cancellation #1907
Conversation
summaryBase version: v3.117.2-0.20251024101740-fda5a9d2c204 (master) |
The test was failing intermittently with "context canceled" error because the 42-second timeout was occasionally exceeded during comprehensive test operations including DB setup, table creation, data insertion, and queries. Increasing to 90 seconds provides adequate buffer for slower CI environments while still catching genuine hangs. Co-authored-by: asmyasnikov <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1907 +/- ##
==========================================
- Coverage 73.98% 73.92% -0.07%
==========================================
Files 390 390
Lines 34154 34154
==========================================
- Hits 25270 25248 -22
- Misses 7765 7781 +16
- Partials 1119 1125 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot every checks require.NoError(t, err) (for example ) need to check root context is cancelled. If root context is cancelled - it is normal that got err on some stage of testIncreasing total timeout not fixed problem with flaky test |
Instead of just increasing timeout, properly handle context cancellation. When the root context is canceled, it's expected that operations fail, so we check ctx.Err() before calling require.NoError(). This follows the pattern used in other integration tests like database_sql_regression_test.go. Co-authored-by: asmyasnikov <[email protected]>
Fixed in commit 34b494a. Reverted the timeout to 42 seconds and added context cancellation checks before all require.NoError calls. The test now skips error assertions when ctx.Err() != nil, following the pattern from database_sql_regression_test.go. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a flaky integration test by properly handling context cancellation scenarios. When the test's shared context times out, subsequent operations fail with expected "context canceled" errors that were being incorrectly treated as test failures.
Key Changes:
- Added conditional error assertions that skip validation when the context is canceled
- Applied the pattern
if ctx.Err() == nil { require.NoError(t, err) }consistently across all test sections
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ctx.Err() == nil { | ||
| require.NoError(t, err) | ||
| var views uint64 | ||
| require.NoError(t, row.Scan(&views)) | ||
| require.EqualValues(t, 1, views) | ||
| } |
There was a problem hiding this comment.
The assertions at lines 354-355 should also be guarded against context cancellation. If ctx.Err() == nil passes but the context is canceled between line 352 and 354, row.Scan(&views) could fail with a context error. Consider checking ctx.Err() before each assertion that could be affected by context state, or move all related assertions inside a single guard.
| if ctx.Err() == nil { | ||
| require.NoError(t, err) | ||
| var ( | ||
| id *uint64 | ||
| title *string | ||
| date *time.Time | ||
| ) | ||
| require.NoError(t, row.Scan(&id, &title, &date)) | ||
| t.Logf( | ||
| " > %d %s %s\n", | ||
| *id, *title, *date, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Similar to Comment 1, the row.Scan() assertion at line 398 is not guarded against context cancellation that could occur after line 391. The same race condition exists where the context could be canceled between the initial check and the scan operation. Consider re-checking ctx.Err() before line 398 or restructuring the logic to handle mid-execution cancellation.
The test intermittently fails with "context canceled" when the timeout is exceeded. The root cause is that the test doesn't properly handle context cancellation - when the shared context times out, subsequent operations fail with "context canceled" errors that are treated as test failures even though they're expected behavior.
Changes
if ctx.Err() == nil) before allrequire.NoError(t, err)calls for operations using the shared contextThis follows the pattern used in other integration tests like
database_sql_regression_test.goand properly handles the scenario where operations fail due to context timeout.Testing
✅ Test compiles successfully
✅ Formatting passes (gofmt)
✅ Follows existing patterns in the codebase
Fixes bug: flaky-test TestBasicExampleQuery #1906
Original prompt
This section details on the original issue you should resolve
<issue_title>bug: flaky-test TestBasicExampleQuery</issue_title>
<issue_description>https://github.com/ydb-platform/ydb-go-sdk/actions/runs/18799671181/job/53645314765
--- FAIL: TestBasicExampleQuery (0.25s)
--- FAIL: TestBasicExampleQuery/ExecuteDataQuery (0.00s)
basic_example_query_test.go:379:
Error Trace: /home/runner/work/ydb-go-sdk/ydb-go-sdk/tests/integration/basic_example_query_test.go:379
Error: Received unexpected error:
pool.With failed with 1 attempts: non-retryable error occurred on attempt No.1 (idempotent=false): context canceled at
github.com/ydb-platform/ydb-go-sdk/v3/internal/query.newResult(result.go:157)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.execute(execute_query.go:142)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).execute(session.go:157)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Session).queryRow(session.go:61)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.clientQueryRow.func1(client.go:313)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.do.func1(client.go:223)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/pool.(*Pool).try(pool.go:465)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/pool.(*Pool).With.func2(pool.go:493)atgithub.com/ydb-platform/ydb-go-sdk/v3/retry.Retry.func1(retry.go:264)atgithub.com/ydb-platform/ydb-go-sdk/v3/retry.opWithRecover(retry.go:418)atgithub.com/ydb-platform/ydb-go-sdk/v3/retry.RetryWithResult(retry.go:358)atgithub.com/ydb-platform/ydb-go-sdk/v3/retry.Retry(retry.go:270)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/pool.(*Pool).With(pool.go:499)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.do(client.go:231)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.clientQueryRow(client.go:319)atgithub.com/ydb-platform/ydb-go-sdk/v3/internal/query.(*Client).QueryRow(client.go:342)Test: TestBasicExampleQuery/ExecuteDataQuery
basic_example_query_test.go:47: total consumed units: 64
mutex.go:15:
[2025-10-25 06:53:36] final registry state:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.