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/robustness: Extract validation to separate package #16066

Merged
merged 1 commit into from
Jun 14, 2023

Conversation

serathius
Copy link
Member

@serathius
Copy link
Member Author

Benjamin is out for 2 weeks, any @etcd-io/members wants to review it?

Copy link
Member

@chaochn47 chaochn47 left a comment

Choose a reason for hiding this comment

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

LGTM with one nit. Thanks @serathius for refactoring the validation part of robustness test that improves the readability !!

@serathius serathius force-pushed the robusness-validate branch 2 times, most recently from 5bb2e1d to 6302ae2 Compare June 13, 2023 19:06
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Happy to have this split out.

One nitpick re consistency of validation functions.

Also need to make fix for goimports.

tests/robustness/validate/watch.go Outdated Show resolved Hide resolved
Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
@serathius
Copy link
Member Author

Also need to make fix for goimports.

make fix doesn't work on my machine (assume some incompatibilities between my local golangci and Go). Sad that fixing imports requires me to fix golangci. @jmhbnz Could we separate make fix-goimport? We had proposed similar effort for contibex.

make fix
./scripts/updatebom.sh
generating bill-of-materials.json
% (cd tools/mod && 'go' 'install' 'github.com/coreos/license-bill-of-materials')
% '/home/serathius/bin/license-bill-of-materials' '--override-file' './bill-of-materials.override.json' 'go.etcd.io/etcd/api/v3/...' 'go.etcd.io/etcd/pkg/v3/...' 'go.etcd.io/etcd/client/pkg/v3/...' 'go.etcd.io/etcd/client/v2/...' 'go.etcd.io/etcd/client/v3/...' 'go.etcd.io/etcd/server/v3/...' 'go.etcd.io/etcd/etcdutl/v3/...' 'go.etcd.io/etcd/etcdctl/v3/...' 'go.etcd.io/etcd/tests/v3/...' 'go.etcd.io/etcd/v3/...'
bom refreshed
golangci-lint run --config tools/.golangci.yaml --fix
tools/benchmark/cmd/lease.go:25:2: `v3` redeclared in this block (typecheck)
        "github.com/cheggaaa/pb/v3"
        ^
tools/benchmark/cmd/lease.go:22:2: other declaration of v3 (typecheck)
        v3 "go.etcd.io/etcd/client/v3"
        ^

@jmhbnz
Copy link
Member

jmhbnz commented Jun 14, 2023

make fix doesn't work on my machine (assume some incompatibilities between my local golangci and Go). Sad that fixing imports requires me to fix golangci. @jmhbnz Could we separate make fix-goimport? We had proposed similar effort for contibex.

Yeah let me take a look at this. Agree we don't want any friction with something so basic.

@serathius serathius merged commit da49157 into etcd-io:main Jun 14, 2023
@serathius serathius deleted the robusness-validate branch June 15, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants