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

Fix tests for latest golang #10666

Merged
merged 1 commit into from May 3, 2019

Conversation

5 participants
@mkumatag
Copy link
Contributor

commented Apr 22, 2019

@mkumatag mkumatag force-pushed the mkumatag:fix_tests branch from b94c351 to 867b45d Apr 22, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Apr 22, 2019

Codecov Report

Merging #10666 into master will increase coverage by 0.3%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #10666     +/-   ##
=========================================
+ Coverage   71.37%   71.67%   +0.3%     
=========================================
  Files         393      393             
  Lines       36629    36629             
=========================================
+ Hits        26145    26255    +110     
+ Misses       8633     8542     -91     
+ Partials     1851     1832     -19
Impacted Files Coverage Δ
etcdserver/api/v3rpc/lease.go 69.31% <0%> (-7.96%) ⬇️
pkg/fileutil/purge.go 65.9% <0%> (-6.82%) ⬇️
etcdctl/ctlv3/command/lease_command.go 65.34% <0%> (-5.95%) ⬇️
clientv3/concurrency/election.go 79.68% <0%> (-2.35%) ⬇️
lease/leasehttp/http.go 64.23% <0%> (-1.46%) ⬇️
proxy/grpcproxy/lease.go 79.18% <0%> (-0.91%) ⬇️
etcdserver/v3_server.go 78.85% <0%> (+0.22%) ⬆️
etcdserver/server.go 74.74% <0%> (+0.35%) ⬆️
etcdserver/api/rafthttp/stream.go 79.82% <0%> (+0.42%) ⬆️
clientv3/client.go 80.11% <0%> (+0.56%) ⬆️
... and 18 more

Continue to review full report at Codecov.

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

@mkumatag

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

@mkumatag

This comment has been minimized.

Copy link
Contributor Author

commented Apr 22, 2019

Tests passed on gotip

@mkumatag

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@dims

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

cc @spzala

@spzala

This comment has been minimized.

Copy link
Member

commented May 3, 2019

@mkumatag so as @hexfusion pointed out in the issue we haven't yet moved to the latest go but the changes looks good to me overall to fix two tests that failed as provided in the issue and per your findings on changes in Errorf. What do you think about https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 for a possible change there as well? I will defer to @hexfusion @xiang90 for final review. Thanks!

@mkumatag

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

@mkumatag so as @hexfusion pointed out in the issue we haven't yet moved to the latest go but the changes looks good to me overall to fix two tests that failed as provided in the issue and per your findings on changes in Errorf. What do you think about https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 for a possible change there as well? I will defer to @hexfusion @xiang90 for final review. Thanks!

@spzala changes in https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 not required because error origin is from https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 is same as where it is originated.

You can check the Travis log for the gotip version in which everything is green, so nothing else we need to worry about!

@spzala

This comment has been minimized.

Copy link
Member

commented May 3, 2019

@mkumatag so as @hexfusion pointed out in the issue we haven't yet moved to the latest go but the changes looks good to me overall to fix two tests that failed as provided in the issue and per your findings on changes in Errorf. What do you think about https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 for a possible change there as well? I will defer to @hexfusion @xiang90 for final review. Thanks!

@spzala changes in https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 not required because error origin is from https://github.com/etcd-io/etcd/blob/master/client/members_test.go#L426 is same as where it is originated.

You can check the Travis log for the gotip version in which everything is green, so nothing else we need to worry about!

@mkumatag cool, thanks! I was wondering about future when we bump the go version so just wanted to make sure that you verify it.

@xiang90

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

lgtm

@xiang90 xiang90 merged commit caee28a into etcd-io:master May 3, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
semaphoreci The build passed on Semaphore.
Details

@mkumatag mkumatag deleted the mkumatag:fix_tests branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.