Skip to content

xtest.Context uses testing.TB.Context() as root context#2046

Open
asmyasnikov wants to merge 7 commits into
masterfrom
context-root
Open

xtest.Context uses testing.TB.Context() as root context#2046
asmyasnikov wants to merge 7 commits into
masterfrom
context-root

Conversation

@asmyasnikov

Copy link
Copy Markdown
Member

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

@asmyasnikov asmyasnikov requested a review from Copilot March 17, 2026 07:27
@github-actions

github-actions Bot commented Mar 17, 2026

Copy link
Copy Markdown

summary

Inferred base version: v3.135.6
Suggested version: v3.135.7

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates pkg/xtest.Context() to derive its root context from testing.TB.Context() (intended for Go 1.24+), so test cancellation/deadlines can propagate naturally into contexts created by xtest.

Changes:

  • Switch xtest.Context(t) to use a contextRoot(t) helper as the parent context.
  • Add Go-version-gated contextRoot(t) implementations via build tags (go1.24 / !go1.24).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
pkg/xtest/context.go Uses contextRoot(t) instead of context.Background() when building the test context.
pkg/xtest/context_root_go1.24.go Adds contextRoot(t) implementation for Go 1.24 builds.
pkg/xtest/context_root.go Adds contextRoot(t) implementation for non-Go-1.24 builds.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread pkg/xtest/context_root.go
Comment thread pkg/xtest/context.go
@codecov-commenter

codecov-commenter commented Mar 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.79%. Comparing base (2d1a2d3) to head (8a69c25).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2046      +/-   ##
==========================================
- Coverage   75.09%   74.79%   -0.31%     
==========================================
  Files         427      428       +1     
  Lines       36918    36920       +2     
==========================================
- Hits        27723    27613     -110     
- Misses       8005     8100      +95     
- Partials     1190     1207      +17     
Flag Coverage Δ
experiment 74.68% <100.00%> (-0.14%) ⬇️
go-1.21.x 72.36% <100.00%> (-0.12%) ⬇️
go-1.26.x 74.76% <100.00%> (-0.28%) ⬇️
integration 57.19% <100.00%> (-0.44%) ⬇️
macOS 46.05% <0.00%> (-0.02%) ⬇️
ubuntu 74.79% <100.00%> (-0.30%) ⬇️
unit 46.07% <0.00%> (-0.01%) ⬇️
windows 46.04% <0.00%> (-0.03%) ⬇️
ydb-24.4 56.79% <100.00%> (-0.15%) ⬇️
ydb-edge 56.73% <100.00%> (-0.38%) ⬇️
ydb-latest 56.67% <100.00%> (-0.22%) ⬇️
ydb-nightly 74.68% <100.00%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@asmyasnikov

Copy link
Copy Markdown
Member Author

@copilot можешь посмотреть почему ломаются тесты если использовать контекст из testing.TB? И надо исправить тесты, т.к. использование контекста из testing.TB - правильное решение

…ntext.Canceled error

Agent-Logs-Url: https://github.com/ydb-platform/ydb-go-sdk/sessions/d0453070-4126-4859-8740-02d862d264c0

Co-authored-by: asmyasnikov <14202262+asmyasnikov@users.noreply.github.com>

Copilot AI commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

@copilot можешь посмотреть почему ломаются тесты если использовать контекст из testing.TB? И надо исправить тесты, т.к...

The tests were failing because in Go 1.24, t.Context() is cancelled before cleanup functions run. So when newTestCommitter's cleanup called res.Close(ctx, ...) with the already-cancelled test context, background.Worker.Close would hit the ctx.Done() branch and return context.Canceled instead of nil or ErrAlreadyClosed.

Fixed in commit 7d977fe by using context.Background() in the cleanup instead of the test context — matching the pattern already used in writer_reconnector_test.go.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • invalid_host
    • Triggering command: /tmp/go-build540175019/b561/credentials.test /tmp/go-build540175019/b561/credentials.test -test.paniconexit0 -test.count=1 -test.timeout=2m0s /tmp/go-build540175019/b494/_pkg_.a s.go go (dns block)
    • Triggering command: /tmp/go-build1458317061/b561/credentials.test /tmp/go-build1458317061/b561/credentials.test -test.paniconexit0 -test.count=1 -test.timeout=3m0s (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants