ronantakizawa/codereview-bench huggingface dataset:
Repository: etcd-io/etcd
File: scripts/codecov_upload.sh (Shell)

Code before:
#!/usr/bin/env bash

# Script used to collect and upload test coverage (mostly by travis).
# Usage ./test_coverage_upload.sh [log_file]

set -o pipefail

# We try to upload whatever we have:
bash <(curl -s https://codecov.io/bash) -f ./covdir/all.coverprofile -cF all


Review comment (suggestion):
This seems to be an unrelated enhancement. It would be better to do it separately. But I won't insist on it.

Code after:
#!/usr/bin/env bash

# Script used to collect and upload test coverage.

set -o pipefail

# We try to upload whatever we have:
mkdir -p bin
curl -sf -o ./bin/codecov.sh https://codecov.io/bash

bash ./bin/codecov.sh -f "${COVERDIR}/all.coverprofile" \
  -cF all \
  -C "${PULL_PULL_SHA}" \
  -r "${REPO_OWNER}/${REPO_NAME}" \
  -P "${PULL_NUMBER}" \
  -b "${BUILD_ID}" \
  -B "${PULL_BASE_REF}" \
  -N "${PULL_BASE_SHA}"
Repository: etcd-io/etcd
File: scripts/markdown_diff_lint.sh (Shell)

Code before:
#!/usr/bin/env bash
# This script runs markdownlint-cli2 on changed files.
# Usage: ./markdown_lint.sh

set -eo pipefail

if ! command markdownlint-cli2 dummy.md &>/dev/null; then
  echo "markdownlint-cli2 needs to be installed."
  echo "Please refer to https://github.com/DavidAnson/markdownlint-cli2?tab=readme-ov-file#install for installation instructions."
  exit 1
fi

COLOR_RED='\033[0;31m'
COLOR_BOLD='\033[1m'
COLOR_ORANGE='\033[0;33m'
COLOR_NONE='\033[0m' # No Color

function log_error {
  echo -n -e "${COLOR_BOLD}${COLOR_RED}$*${COLOR_NONE}\n"
}

function log_warning {
  echo -n -e "${COLOR_ORANGE}$*${COLOR_NONE}\n"
}

if [ -z "${PULL_BASE_SHA}" ]; then
  echo "Empty base reference (\$PULL_BASE_SHA), assuming: main"
  PULL_BASE_SHA=main
fi

if [ -z "${PULL_PULL_SHA}" ]; then
  PULL_PULL_SHA="$(git rev-parse HEAD)"
  echo "Empty pull reference (\$PULL_PULL_SHA), assuming: ${PULL_PULL_SHA}"
fi

Review comment (suggestion):
If we source `test_lib.sh`. These two lines can be changed to `log_errror ...`.

Code after:
#!/usr/bin/env bash
# This script runs markdownlint-cli2 on changed files.
# Usage: ./markdown_lint.sh


# We source ./scripts/test_lib.sh, it sets the log functions and color variables.
source ./scripts/test_lib.sh

# When we source ./scripts/test_lib.sh, it has the line set -u which treats unset variables as errors.
# We need to unset the variable to avoid the error.
set +u -eo pipefail

if ! command markdownlint-cli2 dummy.md &>/dev/null; then
  log_error "markdownlint-cli2 needs to be installed."
  log_error "Please refer to https://github.com/DavidAnson/markdownlint-cli2?tab=readme-ov-file#install for installation instructions."
  exit 1
fi


if [ -z "${PULL_BASE_SHA}" ]; then
  echo "Empty base reference (\$PULL_BASE_SHA), assuming: main"
  PULL_BASE_SHA=main
fi

if [ -z "${PULL_PULL_SHA}" ]; then
  PULL_PULL_SHA="$(git rev-parse HEAD)"
  echo "Empty pull reference (\$PULL_PULL_SHA), assuming: ${PULL_PULL_SHA}"
fi

MD_LINT_URL_PREFIX="https://github.com/DavidAnson/markdownlint/blob/main/doc/"

mapfile -t changed_files < <(git diff "${PULL_BASE_SHA}" --name-only)
declare -A files_with_failures start_ranges end_ranges
Repository: etcd-io/etcd
File: scripts/markdown_diff_lint.sh (Shell)

Code before:
  done
  if [ -z "${ranges}" ]; then
    start_ranges[0]=0
    end_ranges[0]=0
  fi

  i=0
  # Run markdownlint-cli2 with the changed file and print only the summary (stdout).
  markdownlint-cli2 "${file}" 2>/dev/null || true
  while IFS= read -r line; do
    line_number=$(echo "${line}" | awk -F: '{print $2}' | awk '{print $1}')
    while [ "${i}" -lt "${#end_ranges[@]}" ] && [ "${line_number}" -gt "${end_ranges["${i}"]}" ]; do
      i=$((1 + i))
    done
    rule=$(echo "${line}" | gawk 'match($2, /([^\/]+)/, m) {print tolower(m[1])}')
    lint_error="${line} (${MD_LINT_URL_PREFIX}${rule}.md)"

    if [ "${i}" -lt "${#start_ranges[@]}" ] && [ "${line_number}" -ge "${start_ranges["${i}"]}" ] && [ "${line_number}" -le "${end_ranges["${i}"]}" ]; then
      # Inside range with changes, raise an error.
      log_error "${lint_error}"
      files_with_failures["${file}"]=1
    else
      # Outside of range, raise a warning.
      log_warning "${lint_error}"
    fi
  done < <(markdownlint-cli2 "${file}" 2>&1 >/dev/null || true)
done

echo "Finished linting"

for file in "${!files_with_failures[@]}"; do
  log_error "${file} has linting issues"
done
if [ "${#files_with_failures[@]}" -gt "0" ]; then
  exit 1
fi

set -u


Review comment (suggestion):
Same here:

```suggestion
  done < <(markdownlint-cli2 "${file}" --config "${ETCD_ROOT_DIR}/tools/.markdownlint.jsonc" 2>/dev/null || true)
```

Code after:
  done
  if [ -z "${ranges}" ]; then
    start_ranges[0]=0
    end_ranges[0]=0
  fi

  i=0
  # Run markdownlint-cli2 with the changed file and print only the summary (stdout).
  markdownlint-cli2 "${file}" --config "${ETCD_ROOT_DIR}/tools/.markdownlint.jsonc" 2>/dev/null || true
  while IFS= read -r line; do
    line_number=$(echo "${line}" | awk -F: '{print $2}' | awk '{print $1}')
    while [ "${i}" -lt "${#end_ranges[@]}" ] && [ "${line_number}" -gt "${end_ranges["${i}"]}" ]; do
      i=$((1 + i))
    done
    rule=$(echo "${line}" | gawk 'match($2, /([^\/]+)/, m) {print tolower(m[1])}')
    lint_error="${line} (${MD_LINT_URL_PREFIX}${rule}.md)"

    if [ "${i}" -lt "${#start_ranges[@]}" ] && [ "${line_number}" -ge "${start_ranges["${i}"]}" ] && [ "${line_number}" -le "${end_ranges["${i}"]}" ]; then
      # Inside range with changes, raise an error.
      log_error "${lint_error}"
      files_with_failures["${file}"]=1
    else
      # Outside of range, raise a warning.
      log_warning "${lint_error}"
    fi
  done < <(markdownlint-cli2 "${file}" --config "${ETCD_ROOT_DIR}/tools/.markdownlint.jsonc" 2>&1 >/dev/null || true)
done

echo "Finished linting"

for file in "${!files_with_failures[@]}"; do
  log_error "${file} has linting issues"
done
if [ "${#files_with_failures[@]}" -gt "0" ]; then
  exit 1
fi
Repository: etcd-io/etcd
File: server/etcdserver/metrics.go (Go)

Code before:
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "version",
			Help:      "Which version is running. 1 for 'server_version' label with current version.",
		},
		[]string{"server_version"},
	)
	currentGoVersion = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "go_version",
			Help:      "Which Go version server is running with. 1 for 'server_go_version' label with current version.",
		},
		[]string{"server_go_version"},
	)
	serverID = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "id",
			Help:      "Server or member ID in hexadecimal format. 1 for 'server_id' label with current ID.",
		},
		[]string{"server_id"},
	)
	etcdServerFeatureEnabled = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Name: "etcd_server_feature_enabled",
			Help: "Whether or not a feature is enabled. 1 is enabled, 0 is not.",
		},
		[]string{"name", "stage"},
	)
	fdUsed = prometheus.NewGauge(prometheus.GaugeOpts{
		Namespace: "os",
		Subsystem: "fd",
		Name:      "used",
		Help:      "The number of used file descriptors.",
	})
	fdLimit = prometheus.NewGauge(prometheus.GaugeOpts{
		Namespace: "os",
		Subsystem: "fd",
		Name:      "limit",
		Help:      "The file descriptor limit.",
	})
)

func init() {
	prometheus.MustRegister(hasLeader)
	prometheus.MustRegister(isLeader)
	prometheus.MustRegister(leaderChanges)
	prometheus.MustRegister(heartbeatSendFailures)

Review comment (suggestion):
```suggestion
	serverFeatureEnabled = prometheus.NewGaugeVec(
```

Code after:
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "version",
			Help:      "Which version is running. 1 for 'server_version' label with current version.",
		},
		[]string{"server_version"},
	)
	currentGoVersion = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "go_version",
			Help:      "Which Go version server is running with. 1 for 'server_go_version' label with current version.",
		},
		[]string{"server_go_version"},
	)
	serverID = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Namespace: "etcd",
			Subsystem: "server",
			Name:      "id",
			Help:      "Server or member ID in hexadecimal format. 1 for 'server_id' label with current ID.",
		},
		[]string{"server_id"},
	)
	serverFeatureEnabled = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Name: "etcd_server_feature_enabled",
			Help: "Whether or not a feature is enabled. 1 is enabled, 0 is not.",
		},
		[]string{"name", "stage"},
	)
	fdUsed = prometheus.NewGauge(prometheus.GaugeOpts{
		Namespace: "os",
		Subsystem: "fd",
		Name:      "used",
		Help:      "The number of used file descriptors.",
	})
	fdLimit = prometheus.NewGauge(prometheus.GaugeOpts{
		Namespace: "os",
		Subsystem: "fd",
		Name:      "limit",
		Help:      "The file descriptor limit.",
	})
)

func init() {
	prometheus.MustRegister(hasLeader)
	prometheus.MustRegister(isLeader)
	prometheus.MustRegister(leaderChanges)
	prometheus.MustRegister(heartbeatSendFailures)
Repository: etcd-io/etcd
File: etcdctl/ctlv3/command/printer_json.go (Go)

Code before:
// limitations under the License.

package command

import (
	"encoding/json"
	"fmt"

	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
	v3 "go.etcd.io/etcd/client/v3"
)

type jsonPrinter struct {
	isHex bool
	printer
}

func newJSONPrinter(isHex bool) printer {
	return &jsonPrinter{
		isHex:   isHex,
		printer: &printerRPC{newPrinterUnsupported("json"), printJSON},
	}
}

type (
	HexStatusResponse pb.StatusResponse
	HexHashKVResponse pb.HashKVResponse
	HexResponseHeader pb.ResponseHeader
	HexMember         pb.Member
)

func (sr *HexStatusResponse) MarshalJSON() ([]byte, error) {
	type Alias pb.StatusResponse

	return json.Marshal(&struct {
		Header HexResponseHeader `json:"header"`
		Leader string            `json:"leader"`
		Alias
	}{
		Header: (HexResponseHeader)(*sr.Header),
		Leader: fmt.Sprintf("%x", sr.Leader),
		Alias:  (Alias)(*sr),
	})
}

func (hr *HexHashKVResponse) MarshalJSON() ([]byte, error) {
	type Alias pb.HashKVResponse

	return json.Marshal(&struct {
		Header HexResponseHeader `json:"header"`
		Alias

Review comment (bug):
Thanks for filing this refactor change.
Would you please add one method `MemberAdd` to `jsonPrinter` to fix that issue first?
Basically, we don't involve refactor change in bugfix. It's easy to review and maintain.
Thanks!

Code after:
// limitations under the License.

package command

import (
	"encoding/json"
	"fmt"
	"io"
	"os"
	"strconv"
	"strings"

	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
	clientv3 "go.etcd.io/etcd/client/v3"
)

type jsonPrinter struct {
	writer io.Writer
	isHex  bool
	printer
}

type (
	HexResponseHeader pb.ResponseHeader
	HexMember         pb.Member
)

func (h *HexResponseHeader) MarshalJSON() ([]byte, error) {
	type Alias pb.ResponseHeader

	return json.Marshal(&struct {
		ClusterID string `json:"cluster_id"`
		MemberID  string `json:"member_id"`
		Alias
	}{
		ClusterID: fmt.Sprintf("%x", h.ClusterId),
		MemberID:  fmt.Sprintf("%x", h.MemberId),
		Alias:     (Alias)(*h),
	})
}

func (m *HexMember) MarshalJSON() ([]byte, error) {
	type Alias pb.Member

	return json.Marshal(&struct {
		ID string `json:"ID"`
		Alias
	}{
		ID:    fmt.Sprintf("%x", m.ID),
		Alias: (Alias)(*m),
	})
Repository: etcd-io/etcd
File: etcdctl/ctlv3/command/printer_json.go (Go)

Code before:
		Alias: (Alias)(*m),
	})
}

func newJSONPrinter(isHex bool) printer {
	return &jsonPrinter{
		isHex:   isHex,
		printer: &printerRPC{newPrinterUnsupported("json"), printJSON},
	}
}

func (p *jsonPrinter) EndpointHealth(r []epHealth) { printJSON(r) }
func (p *jsonPrinter) EndpointStatus(r []epStatus) { printJSON(r) }
func (p *jsonPrinter) EndpointHashKV(r []epHashKV) { printJSON(r) }

func (p *jsonPrinter) MemberList(r clientv3.MemberListResponse) {
	if p.isHex {
		printMemberListWithHexJSON(r)
	} else {
		printJSON(r)
	}
}
func (p *jsonPrinter) MemberAdd(r clientv3.MemberAddResponse) { p.printJSON(r) }

func printJSON(v any) {
	b, err := json.Marshal(v)
	if err != nil {
		fmt.Fprintf(os.Stderr, "%v\n", err)
		return
	}
	fmt.Println(string(b))
}

func printMemberListWithHexJSON(r clientv3.MemberListResponse) {
	var buffer strings.Builder
	var b []byte
	buffer.WriteString("{\"header\":{\"cluster_id\":\"")
	b = strconv.AppendUint(nil, r.Header.ClusterId, 16)
	buffer.Write(b)
	buffer.WriteString("\",\"member_id\":\"")
	b = strconv.AppendUint(nil, r.Header.MemberId, 16)
	buffer.Write(b)
	buffer.WriteString("\",\"raft_term\":")
	b = strconv.AppendUint(nil, r.Header.RaftTerm, 10)
	buffer.Write(b)
	buffer.WriteByte('}')
	for i := 0; i < len(r.Members); i++ {
		if i == 0 {
			buffer.WriteString(",\"members\":[{\"ID\":\"")
		} else {
			buffer.WriteString(",{\"ID\":\"")

Review comment (suggestion):
I think we can construct a fake `MemberAddResponse` object, and call this method directly to ensure that it generates the expected output with and without `--hex`.

We also need to make similar change for some other commands. So let's make this PR as a good example for following PRs to follow.

Code after:
	}{
		ID:    fmt.Sprintf("%x", m.ID),
		Alias: (Alias)(*m),
	})
}

func newJSONPrinter(isHex bool) printer {
	return &jsonPrinter{
		writer:  os.Stdout,
		isHex:   isHex,
		printer: &printerRPC{newPrinterUnsupported("json"), printJSON},
	}
}

func (p *jsonPrinter) EndpointHealth(r []epHealth) { printJSON(r) }
func (p *jsonPrinter) EndpointStatus(r []epStatus) { printJSON(r) }
func (p *jsonPrinter) EndpointHashKV(r []epHashKV) { printJSON(r) }

func (p *jsonPrinter) MemberList(r clientv3.MemberListResponse) {
	if p.isHex {
		printMemberListWithHexJSON(p.writer, r)
	} else {
		printJSON(r)
	}
}
func (p *jsonPrinter) MemberAdd(r clientv3.MemberAddResponse) { p.printJSON(r) }

func printJSONTo(w io.Writer, v any) {
	b, err := json.Marshal(v)
	if err != nil {
		fmt.Fprintf(os.Stderr, "%v\n", err)
		return
	}
	fmt.Fprintln(w, string(b))
}

func printJSON(v any) {
	printJSONTo(os.Stdout, v)
}

func printMemberListWithHexJSON(w io.Writer, r clientv3.MemberListResponse) {
	var buffer strings.Builder
	var b []byte
	buffer.WriteString("{\"header\":{\"cluster_id\":\"")
	b = strconv.AppendUint(nil, r.Header.ClusterId, 16)
	buffer.Write(b)
	buffer.WriteString("\",\"member_id\":\"")
	b = strconv.AppendUint(nil, r.Header.MemberId, 16)
	buffer.Write(b)
	buffer.WriteString("\",\"raft_term\":")
	b = strconv.AppendUint(nil, r.Header.RaftTerm, 10)
Repository: etcd-io/etcd
File: server/etcdserver/api/membership/cluster_test.go (Go)

Code before:
	now := time.Now()
	alice := NewMember("", nil, "alice", &now)

	tcs := []struct {
		name string

		storeV2Nil     bool
		backendNil     bool
		storeV2Members []*Member
		backendMembers []*Member

		expectPanics  bool
		expectMembers map[types.ID]*Member
	}{
		{
			name: "Adding new member should succeed",
		},
		{
			name:           "Adding member should succeed if it was only in storeV2",
			storeV2Members: []*Member{alice},
		},
		{
			name:           "Adding member should succeed if it was only in backend",
			backendMembers: []*Member{alice},
		},
		{
			name:           "Adding member should fail if it exists in storeV2 and backend is nil",
			storeV2Members: []*Member{alice},
			backendNil:     true,
			expectPanics:   true,
		},
		{
			name:           "Adding member should succeed if it exists in backend and storageV2 is nil",
			storeV2Nil:     true,
			backendMembers: []*Member{alice},
		},
		{
			name:           "Adding new member should succeed if backend is nil",
			storeV2Members: []*Member{},
			backendNil:     true,
		},
		{
			name:           "Adding new member should fail if storageV2 is nil",
			storeV2Nil:     true,
			backendMembers: []*Member{},
		},
	}
	for _, tc := range tcs {
		t.Run(tc.name, func(t *testing.T) {
			lg := zaptest.NewLogger(t)
			be, _ := betesting.NewDefaultTmpBackend(t)

Review comment (bug):
Reason for this test was to confirm that we will still return error when we try to add an existing member to cluster. The bug came from fact that we forgot about learners and setting attributes, however I think we should have an equivalent test.

Code after:
	now := time.Now()
	alice := NewMember("", nil, "alice", &now)

	tcs := []struct {
		name string

		storeV2Nil     bool
		backendNil     bool
		storeV2Members []*Member
		backendMembers []*Member

		expectPanics  bool
		expectMembers map[types.ID]*Member
	}{
		{
			name: "Adding new member should succeed",
		},
		{
			name:           "Adding member should succeed if it was only in storeV2",
			storeV2Members: []*Member{alice},
		},
		{
			name:           "Adding member should succeed if it was only in backend",
			backendMembers: []*Member{alice},
		},
		{
			name:           "Adding member should success if it exists in both",
			storeV2Members: []*Member{alice},
			backendMembers: []*Member{alice},
		},
		{
			name:           "Adding member should fail if it exists in storeV2 and backend is nil",
			storeV2Members: []*Member{alice},
			backendNil:     true,
			expectPanics:   true,
		},
		{
			name:           "Adding member should succeed if it exists in backend and storageV2 is nil",
			storeV2Nil:     true,
			backendMembers: []*Member{alice},
		},
		{
			name:           "Adding new member should succeed if backend is nil",
			storeV2Members: []*Member{},
			backendNil:     true,
		},
		{
			name:           "Adding new member should fail if storageV2 is nil",
			storeV2Nil:     true,
			backendMembers: []*Member{},
		},
Repository: etcd-io/etcd
File: tests/e2e/ctl_v2_test.go (Go)

Code before:

	if err := etcdctlRoleAdd(epc, "foo"); err != nil {
		t.Fatalf("failed to add role (%v)", err)
	}
	if err := etcdctlRoleList(epc, "foo"); err != nil {
		t.Fatalf("failed to list roles (%v)", err)
	}
}

func TestUtlCtlV2Backup(t *testing.T) {
	for snap := range []int{0, 1} {
		for _, v3 := range []bool{true, false} {
			for _, utl := range []bool{true, false} {
				t.Run(fmt.Sprintf("etcdutl:%v;snap:%v;v3:%v", utl, snap, v3),
					func(t *testing.T) {
						testUtlCtlV2Backup(t, snap, v3, utl)
					})
			}
		}
	}
}

func testUtlCtlV2Backup(t *testing.T, snapCount int, v3 bool, utl bool) {
	BeforeTestV2(t)

	// disable the verification because this case offline updated the db
	revertFunc := verify.DisableVerification()
	defer revertFunc()

	backupDir, err := ioutil.TempDir(t.TempDir(), "testbackup0.etcd")
	if err != nil {
		t.Fatal(err)
	}

	etcdCfg := e2e.NewConfigNoTLS()
	etcdCfg.SnapshotCount = snapCount
	etcdCfg.EnableV2 = true
	t.Log("Starting etcd-1")
	epc1 := setupEtcdctlTest(t, etcdCfg, false)

	// v3 put before v2 set so snapshot happens after v3 operations to confirm
	// v3 data is preserved after snapshot.
	os.Setenv("ETCDCTL_API", "3")
	if err := ctlV3Put(ctlCtx{t: t, epc: epc1}, "v3key", "123", ""); err != nil {
		t.Fatal(err)
	}
	os.Setenv("ETCDCTL_API", "2")

	t.Log("Setting key in etcd-1")
	if err := etcdctlSet(epc1, "foo1", "bar1"); err != nil {
		t.Fatal(err)

Review comment (suggestion):
```suggestion
	// disable the verification because this case updated the bbolt db offline
```

Code after:

	if err := etcdctlRoleAdd(epc, "foo"); err != nil {
		t.Fatalf("failed to add role (%v)", err)
	}
	if err := etcdctlRoleList(epc, "foo"); err != nil {
		t.Fatalf("failed to list roles (%v)", err)
	}
}

func TestUtlCtlV2Backup(t *testing.T) {
	for snap := range []int{0, 1} {
		for _, v3 := range []bool{true, false} {
			for _, utl := range []bool{true, false} {
				t.Run(fmt.Sprintf("etcdutl:%v;snap:%v;v3:%v", utl, snap, v3),
					func(t *testing.T) {
						testUtlCtlV2Backup(t, snap, v3, utl)
					})
			}
		}
	}
}

func testUtlCtlV2Backup(t *testing.T, snapCount int, v3 bool, utl bool) {
	BeforeTestV2(t)

	// disable the verification because this case updated the db offline
	revertFunc := verify.DisableVerification()
	defer revertFunc()

	backupDir, err := ioutil.TempDir(t.TempDir(), "testbackup0.etcd")
	if err != nil {
		t.Fatal(err)
	}

	etcdCfg := e2e.NewConfigNoTLS()
	etcdCfg.SnapshotCount = snapCount
	etcdCfg.EnableV2 = true
	t.Log("Starting etcd-1")
	epc1 := setupEtcdctlTest(t, etcdCfg, false)

	// v3 put before v2 set so snapshot happens after v3 operations to confirm
	// v3 data is preserved after snapshot.
	os.Setenv("ETCDCTL_API", "3")
	if err := ctlV3Put(ctlCtx{t: t, epc: epc1}, "v3key", "123", ""); err != nil {
		t.Fatal(err)
	}
	os.Setenv("ETCDCTL_API", "2")

	t.Log("Setting key in etcd-1")
	if err := etcdctlSet(epc1, "foo1", "bar1"); err != nil {
		t.Fatal(err)
Repository: etcd-io/etcd
File: tests/robustness/client/kvhash.go (Go)

Code before:
// limitations under the License.

package client

import (
	"context"
	"testing"
	"time"

	"github.com/stretchr/testify/require"

	clientv3 "go.etcd.io/etcd/client/v3"
	"go.etcd.io/etcd/tests/v3/framework/e2e"
	"go.etcd.io/etcd/tests/v3/robustness/identity"
	"go.etcd.io/etcd/tests/v3/robustness/report"
)

func CheckHashKV(ctx context.Context, t *testing.T, clus *e2e.EtcdProcessCluster, rev int64, baseTime time.Time, ids identity.Provider) []report.ClientReport {
	reports := make([]report.ClientReport, len(clus.Procs))
	HashKVs := make([]*clientv3.HashKVResponse, len(clus.Procs))
	for i, member := range clus.Procs {
		c, err := NewRecordingClient(member.EndpointsGRPC(), ids, baseTime)
		require.NoError(t, err)
		defer c.Close()

		hash, err := c.HashKV(ctx, rev)
		require.NoError(t, err)
		require.Lenf(t, hash, 1, "We should only have 1 HashKVResponse per request")
		HashKVs[i] = hash[0]

		reports[i] = c.Report()
	}

	for i := 1; i < len(clus.Procs); i++ {
		require.Equalf(t, HashKVs[i-1].HashRevision, HashKVs[i].HashRevision, "HashRevision mismatch")
		require.Equalf(t, HashKVs[i-1].CompactRevision, HashKVs[i].CompactRevision, "CompactRevision mismatch")
		require.Equalf(t, HashKVs[i-1].Hash, HashKVs[1].Hash, "Hash mismatch")
	}

	return reports
}


Review comment (suggestion):
This doesn't support `revision 0`, to make this function work with it you need to make a `HashKV` request with `rev:0` to one member, read the returned revision and then make `HashKV` with this revision to other members. 

Code after:
// limitations under the License.

package client

import (
	"context"
	"fmt"
	"time"

	"go.uber.org/zap"

	clientv3 "go.etcd.io/etcd/client/v3"
	"go.etcd.io/etcd/tests/v3/framework/e2e"
)

func CheckHashKV(ctx context.Context, clus *e2e.EtcdProcessCluster) error {
	c, err := clientv3.New(clientv3.Config{
		Endpoints:            clus.EndpointsGRPC(),
		Logger:               zap.NewNop(),
		DialKeepAliveTime:    10 * time.Second,
		DialKeepAliveTimeout: 100 * time.Millisecond,
	})
	if err != nil {
		return err
	}
	defer c.Close()

	hashKV, err := c.HashKV(ctx, clus.EndpointsGRPC()[0], 0)
	if err != nil {
		return err
	}
	rev := hashKV.Header.Revision

	hashKVs := make([]*clientv3.HashKVResponse, 0)
	hashKVs = append(hashKVs, hashKV)

	for _, member := range clus.Procs {
		hashKV, err := c.HashKV(ctx, member.EndpointsGRPC()[0], rev)
		if err != nil {
			return err
		}
		if hashKV.Header.Revision != rev {
			return fmt.Errorf("max revision between nodes should be the same. Want %v, get %v", rev, hashKV.Header.Revision)
		}
		hashKVs = append(hashKVs, hashKV)
	}

	for i := 1; i < len(hashKVs); i++ {
		if hashKVs[i-1].HashRevision != hashKVs[i].HashRevision {
			return fmt.Errorf("hashRevision mismatch, node %v has %+v, node %v has %+v", hashKVs[i-1].Header.MemberId, hashKVs[i-1], hashKVs[i].Header.MemberId, hashKVs[i])
		}