Skip to content

Conversation

yagikota
Copy link
Contributor

@yagikota yagikota commented Aug 10, 2025

What?

Migrate e2e(tests/e2e/ctl_v3_grpc_test.go) and integration(tests/integration/grpc_test.go) tests to the common framework

  • Add tests/common/grpc_test.go: unified TestAuthority that delegates to the active runner

  • Introduce runner interfaces in tests/framework/interfaces/interface.go: TemplateEndpoints, AssertAuthority

  • E2E runner updates:

    • Extend ClusterContext with EnvVars and UseUnix in tests/framework/e2e/config.go
    • Wire EnvVars["GODEBUG"]=http2debug=2 and UseUnix handling in tests/framework/e2e/e2e.go
    • Implement TemplateEndpoints and AssertAuthority (assert via http2 header logs)
  • Integration runner updates:

    • Add ClusterContext{UseUnix} in tests/framework/integration/config.go
    • Respect UseUnix (flip UseTCP/UseIP) in tests/framework/integration/integration.go
    • Implement TemplateEndpoints and AssertAuthority (assert via RecordedRequests() filter on /etcdserverpb.KV/Put)
  • Add common helpers:

    • WithHTTP2Debug, WithUnixClient, WithTCPClient in:
      • tests/common/e2e_test.go
      • tests/common/integration_test.go
      • tests/common/unit_test.go (no-ops where appropriate)

Why?

Test Result

E2E

Before Migration

cd etcd/tests/e2e
go test -v -count=1 -run TestAuthority -args -bin-dir=[directory for store etcd binary]
...
--- PASS: TestAuthority (101.62s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix:path" (2.95s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix:path" (5.53s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix://absolute_path" (3.09s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix://absolute_path" (5.41s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs:absolute_path" (3.11s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs:absolute_path" (6.06s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs://absolute_path" (2.78s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs://absolute_path" (7.45s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://domain[:port]" (3.25s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://domain[:port]" (6.84s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://address[:port]" (3.27s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://address[:port]" (6.58s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]_insecure" (4.49s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]_insecure" (8.75s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]_insecure" (3.51s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]_insecure" (9.22s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]" (3.05s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]" (6.19s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]" (2.86s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]" (7.23s)
PASS
ok  	go.etcd.io/etcd/tests/v3/e2e	101.931s

After Migration

cd etcd/tests/common
go test --tags=e2e -v -count=1 -run TestAuthority -args -bin-dir=[directory for store etcd binary]
...
--- PASS: TestAuthority (98.74s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix:path" (2.75s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix:path" (5.55s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix://absolute_path" (3.02s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix://absolute_path" (5.43s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs:absolute_path" (3.23s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs:absolute_path" (7.00s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs://absolute_path" (2.76s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs://absolute_path" (6.94s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://domain[:port]" (3.38s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://domain[:port]" (7.96s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://address[:port]" (3.47s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://address[:port]" (5.53s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]_insecure" (4.37s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]_insecure" (6.27s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]_insecure" (3.45s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]_insecure" (6.29s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]" (3.46s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]" (8.17s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]" (3.25s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]" (6.45s)
PASS
ok  	go.etcd.io/etcd/tests/v3/common	99.034s

Integration

Before Migration

cd etcd/tests/integration
go test -v -count=1 -run TestAuthority
...
--- PASS: TestAuthority (24.33s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix:path" (0.87s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix:path" (1.99s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix://absolute_path" (0.86s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix://absolute_path" (2.46s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs:absolute_path" (0.82s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs:absolute_path" (2.28s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs://absolute_path" (0.89s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs://absolute_path" (2.14s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://domain[:port]" (0.88s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://domain[:port]" (1.89s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]" (0.89s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]" (2.19s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://address[:port]" (0.88s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://address[:port]" (2.21s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]" (0.90s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]" (2.16s)
PASS
ok  	go.etcd.io/etcd/tests/v3/integration	24.675s

After Migration

cd etcd/tests/common
go test --tags=integration -v -count=1 -run TestAuthority
...
--- PASS: TestAuthority (33.01s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix:path" (0.85s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix:path" (1.98s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unix://absolute_path" (0.84s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unix://absolute_path" (2.02s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs:absolute_path" (0.79s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs:absolute_path" (2.06s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"unixs://absolute_path" (0.80s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"unixs://absolute_path" (2.46s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://domain[:port]" (0.88s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://domain[:port]" (2.50s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"http://address[:port]" (0.90s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"http://address[:port]" (2.66s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]_insecure" (1.01s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]_insecure" (2.67s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]_insecure" (1.01s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]_insecure" (2.55s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://domain[:port]" (0.99s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://domain[:port]" (2.56s)
    --- PASS: TestAuthority/Size:_1,_Scenario:_"https://address[:port]" (0.92s)
    --- PASS: TestAuthority/Size:_3,_Scenario:_"https://address[:port]" (2.55s)
PASS
ok  	go.etcd.io/etcd/tests/v3/common	33.327s

Ref

@k8s-ci-robot
Copy link

Hi @yagikota. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@yagikota yagikota changed the title Migrate grpc tests to common framework [WIP]Migrate grpc tests to common framework Aug 10, 2025
@yagikota yagikota force-pushed the 1367-migrate-e2e-ctl-v3-grpc-test branch from 1c8cf5a to 14c7696 Compare August 11, 2025 10:07
@yagikota yagikota force-pushed the 1367-migrate-e2e-ctl-v3-grpc-test branch from 14c7696 to 9a737f1 Compare August 11, 2025 10:11
@yagikota yagikota force-pushed the 1367-migrate-e2e-ctl-v3-grpc-test branch from 9a737f1 to de2ae5d Compare August 11, 2025 14:15
@yagikota yagikota marked this pull request as ready for review August 11, 2025 14:57
@yagikota yagikota requested a review from serathius August 11, 2025 14:57
@yagikota yagikota changed the title [WIP]Migrate grpc tests to common framework tests: Migrate grpc tests(e2e/integration) to the common framework Aug 11, 2025
@serathius
Copy link
Member

/ok-to-test

@serathius
Copy link
Member

Great work! Impressive for new contributor, thank you.

Copy link

codecov bot commented Aug 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.14%. Comparing base (3f733e2) to head (1f7163f).
⚠️ Report is 41 commits behind head on main.

Additional details and impacted files

see 53 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20464      +/-   ##
==========================================
- Coverage   69.25%   69.14%   -0.12%     
==========================================
  Files         417      418       +1     
  Lines       34741    34699      -42     
==========================================
- Hits        24061    23991      -70     
- Misses       9288     9310      +22     
- Partials     1392     1398       +6     

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f733e2...1f7163f. Read the comment docs.

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

@serathius
Copy link
Member

Please fix tests

@yagikota
Copy link
Contributor Author

Hmm, still errors...

@yagikota
Copy link
Contributor Author

yagikota commented Aug 13, 2025

@serathius

FI fixed all CI errors.
Would you merge this PR if you think it's ok?

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: serathius, yagikota

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@serathius
Copy link
Member

Can you squash commits before merge?

@yagikota yagikota force-pushed the 1367-migrate-e2e-ctl-v3-grpc-test branch from 5a16577 to 1f7163f Compare August 13, 2025 13:53
@yagikota
Copy link
Contributor Author

Can you squash commits before merge?

Done.

@yagikota yagikota requested a review from serathius August 13, 2025 13:54
@serathius serathius merged commit a839618 into etcd-io:main Aug 13, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants