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

tests: Migrate member list tests to common framework #14278

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

clement2026
Copy link
Contributor

Context #13637

Signed-off-by: Clark fwyongxing@gmail.com

@serathius
Copy link
Member

I would recommend to not do everything in one PR. If you can migrate one test independently it would be faster for us to iterate.

@clement2026 clement2026 changed the title tests: Migrate member tests to common framework tests: Migrate member list tests to common framework Jul 26, 2022
@clement2026
Copy link
Contributor Author

I would recommend to not do everything in one PR. If you can migrate one test independently it would be faster for us to iterate.

sounds good.
I've made some changes: In this PR, I'll only migrate member list tests.

Copy link
Member

@serathius serathius left a comment

Choose a reason for hiding this comment

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

This PR makes me super excited. With only couple of lines the test is much more readable as we reuse so much code.

For context this is the old test:

func ctlV3MemberList(cx ctlCtx) error {
	cmdArgs := append(cx.PrefixArgs(), "member", "list")
	lines := make([]string, cx.cfg.ClusterSize)
	for i := range lines {
		lines[i] = "started"
	}
	return e2e.SpawnWithExpects(cmdArgs, cx.envMap, lines...)
}

@serathius
Copy link
Member

DCO failed after adding the suggestion commit. Can you squash the commits in PR?

Signed-off-by: Clark <fwyongxing@gmail.com>
@clement2026
Copy link
Contributor Author

DCO failed after adding the suggestion commit. Can you squash the commits in PR?

Sure

@codecov-commenter
Copy link

Codecov Report

Merging #14278 (13d20f8) into main (a3b410c) will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #14278      +/-   ##
==========================================
- Coverage   75.52%   75.35%   -0.18%     
==========================================
  Files         456      456              
  Lines       37037    37037              
==========================================
- Hits        27973    27909      -64     
- Misses       7322     7373      +51     
- Partials     1742     1755      +13     
Flag Coverage Δ
all 75.35% <ø> (-0.18%) ⬇️

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

Impacted Files Coverage Δ
client/pkg/v3/fileutil/lock_linux.go 72.22% <0.00%> (-8.34%) ⬇️
client/pkg/v3/fileutil/purge.go 66.03% <0.00%> (-7.55%) ⬇️
client/v3/namespace/watch.go 87.87% <0.00%> (-6.07%) ⬇️
raft/rafttest/node.go 95.00% <0.00%> (-5.00%) ⬇️
server/storage/mvcc/watchable_store.go 85.14% <0.00%> (-4.72%) ⬇️
server/storage/wal/file_pipeline.go 90.69% <0.00%> (-4.66%) ⬇️
server/proxy/grpcproxy/watch.go 92.48% <0.00%> (-4.05%) ⬇️
server/etcdserver/txn/util.go 75.47% <0.00%> (-3.78%) ⬇️
server/etcdserver/api/v3rpc/interceptor.go 74.47% <0.00%> (-3.13%) ⬇️
client/v3/concurrency/election.go 79.68% <0.00%> (-2.35%) ⬇️
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@serathius serathius merged commit c04c65c into etcd-io:main Jul 26, 2022
@clement2026 clement2026 deleted the migrate-member-tests branch July 28, 2022 08:31
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