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

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

# This scripts runs benchmark tests on the locally spun-up etcd instance

set -euo pipefail

source ./scripts/test_lib.sh

declare -A BENCHMARK_FLAGS
BENCHMARKS_TO_RUN=()
COMMON_BENCHMARK_FLAGS="--report-perfdash"

# Supported benchmarks
ALL_BENCHMARKS=(put lease-keepalive range stm txn-mixed txn-put watch watch-get watch-latency)

for arg in "$@"; do
  if [[ "$arg" == *:* ]]; then
    key="${arg%%:*}"
    value="${arg#*:}"
    BENCHMARKS_TO_RUN+=("$key")
    BENCHMARK_FLAGS["$key"]="$value"
  else
    BENCHMARKS_TO_RUN+=("$arg")
    BENCHMARK_FLAGS["$arg"]=""
  fi
done

# If there are no specific benchmarks mentioned, run benchmarks of all types apart from mvcc.
if [ ${#BENCHMARKS_TO_RUN[@]} -eq 0 ]; then
  BENCHMARKS_TO_RUN=("${ALL_BENCHMARKS[@]}")
fi

# Set default "key" flag only if benchmark range needs to be run and no flags were passed
if [[ " ${BENCHMARKS_TO_RUN[*]} " == *" range "* ]] && [[ -z "${BENCHMARK_FLAGS[range]+set}" ]]; then
  BENCHMARK_FLAGS[range]="key"
fi

echo "Starting the etcd server..."
./bin/etcd > /tmp/etcd.log 2>&1 &

Review comment (performance):
You focused on trying to run all benchmarks which lead to overgeneralization. We wanted to just start from one good benchmark and integrate it e2e. Doing them all at once will slow down development.   

Code after:
#!/usr/bin/env bash
# Copyright 2025 The etcd Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script runs a benchmark on a locally started etcd server

set -euo pipefail

source ./scripts/test_lib.sh

COMMON_BENCHMARK_FLAGS="--report-perfdash"

if [[ $# -lt 1 ]]; then
  echo "Usage: $0 <benchmark-name> [tester args...]"
  exit 1
fi

BENCHMARK_NAME="$1"
ARGS="${*:2}"

echo "Starting the etcd server..."
DATA_DIR=$(mktemp -d /tmp/etcd/data-XXXXXX)
./bin/etcd --data-dir="$DATA_DIR" > /tmp/etcd.log 2>&1 &
etcd_pid=$!

trap 'log_warning -e "Stopping etcd server - PID $etcd_pid";
      kill $etcd_pid 2>/dev/null;
      rm -rf $DATA_DIR;
Repository: etcd-io/etcd
File: scripts/benchmark_test.sh (Shell)

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

# This script runs a benchmark on a locally started etcd server

set -euo pipefail

source ./scripts/test_lib.sh

COMMON_BENCHMARK_FLAGS="--report-perfdash"

if [[ $# -lt 1 ]]; then
  echo "Usage: $0 <benchmark-name> [tester args...]"
  exit 1
fi

BENCHMARK_NAME="$1"
ARGS="${*:2}"

echo "Starting the etcd server..."
DATA_DIR=$(mktemp -d /tmp/etcd/data-XXXXXX)
./bin/etcd --data-dir="$DATA_DIR" > /tmp/etcd.log 2>&1 &
etcd_pid=$!

trap 'log_warning -e "Stopping etcd server - PID $etcd_pid";
      kill $etcd_pid 2>/dev/null;
      rm -rf $DATA_DIR;

Review comment (security):
```suggestion
#!/usr/bin/env bash
# Copyright 2025 The etcd Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
```

Code after:
#!/usr/bin/env bash
# Copyright 2025 The etcd Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script runs a benchmark on a locally started etcd server

set -euo pipefail

source ./scripts/test_lib.sh

COMMON_BENCHMARK_FLAGS="--report-perfdash"

if [[ $# -lt 1 ]]; then
  echo "Usage: $0 <benchmark-name> [tester args...]"
  exit 1
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(f func()) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		f()
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}

func VerifyPrototype(p func() (bool, string, map[string]any)) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, msg, details := p()
		verifier(ok, msg, details)
	}
}

func verifier(condition bool, msg string, _ map[string]any) {
	if condition {
		panic(msg)
	}
}


Review comment (suggestion):
Create a type for function `p`, like:
```
type VerifyFunc func() (ok bool, name string, details map[string]any)
```

Code after:
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
	// import_placeholder
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()
	os.Setenv(envVerify, string(verification))

Review comment (suggestion):
Please make it more clear reason to have placeholder and why it should be removed.
Like 
```
// NO NOT REMOVE; used to replace panic with Antithesis assertions to validate etcd on their platform.
// antithesis_import_assert_placeholder
```

Code after:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()
	os.Setenv(envVerify, string(verification))
	return func() {
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if condition {
		panic(fmt.Sprintf("%s. details %v.", msg, details))
	}
}


Review comment (suggestion):
Please leave a comment  that would document that `verifier` name should not be changed as it is replaced with Antithesis assertions when validating etcd on their platform.

Code after:
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()
	os.Setenv(envVerify, string(verification))
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}

const PostActionFunc = "POST_ACTION"

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		// the function name should not be changed. It will be replaced with Antithesis assertions
		// when validating etcd on their platform.
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	f := func() { panic(fmt.Sprintf("%s. details: %v.", msg, details)) }
	if postAction, ok := details[PostActionFunc]; ok {
		if action, ok := postAction.(func()); ok {

Review comment (suggestion):
We should replace all calls to `Assert` with verify and remove it.

Code after:
	os.Setenv(envVerify, string(verification))
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// EnableAllVerifications enables verification and returns a function
// that can be used to bring the original settings.
func EnableAllVerifications() func() {
	return EnableVerifications(envVerifyValueAll)
}

// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// DisableVerifications unsets `envVerify` and returns a function that
// can be used to bring the original settings.
func DisableVerifications() func() {
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}

const PostActionFunc = "POST_ACTION"

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		// the function name should not be changed. It will be replaced with Antithesis assertions
		// when validating etcd on their platform.
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	f := func() { panic(fmt.Sprintf("%s. details: %v.", msg, details)) }
	if postAction, ok := details[PostActionFunc]; ok {
		if action, ok := postAction.(func()); ok {
			f = action
		}
	}
	if condition {
		f()
	}
}


Review comment (suggestion):
```suggestion
		// DO NOT CHANGE; the line below is replaced with Antithesis assertions  when validating etcd on their platform.
		// antithesis_verifier_placeholder
```

Code after:
	previousEnv := getEnvVerify()
	os.Unsetenv(envVerify)
	return func() {
		os.Setenv(envVerify, previousEnv)
	}
}

// Verify performs verification if the assertions are enabled.
// In the default setup running in tests and skipped in the production code.
func Verify(msg string, f VerifyFunc) {
	if IsVerificationEnabled(envVerifyValueAssert) {
		ok, details := f()
		verifier(ok, msg, details)
	}
}

type VerifyFunc func() (condition bool, details map[string]any)

func verifier(condition bool, msg string, details map[string]any) {
	if !condition {
		panic(fmt.Sprintf("%s. details: %v.", msg, details))
	}
}

// Assert will panic with a given formatted message if the given condition is false.
func Assert(condition bool, msg string, v ...any) {
	if !condition {
		panic(fmt.Sprintf("assertion failed: "+msg, v...))
	}
}
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
	// DO NOT REMOVE; will be replaced by the Antithesis assertion package when
	// building etcd-server for running on the Antithesis platform.
	// antithesis_import_assert_placeholder
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()

Review comment (suggestion):
```suggestion
	// DO NOT REMOVE; will be replaced by the Antithesis assertion package when building etcd-server for running on the Antithesis platform.
```

Code after:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()
	os.Setenv(envVerify, string(verification))
	return func() {
		os.Setenv(envVerify, previousEnv)
Repository: etcd-io/etcd
File: client/pkg/verify/verify.go (Go)

Code before:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
	// DO NOT REMOVE; will be replaced by the Antithesis assertion package when building etcd-server for running on the Antithesis platform.
	// antithesis_import_assert_placeholder
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()

Review comment (suggestion):
```suggestion
	// DO NOT CHANGE LINE BELOW; it is replaced with Antithesis assertions when validating etcd on their platform
```

Code after:
// Copyright 2022 The etcd Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package verify

import (
	"fmt"
	"os"
	"strings"
)

const envVerify = "ETCD_VERIFY"

type VerificationType string

const (
	envVerifyValueAll    VerificationType = "all"
	envVerifyValueAssert VerificationType = "assert"
)

func getEnvVerify() string {
	return strings.ToLower(os.Getenv(envVerify))
}

func IsVerificationEnabled(verification VerificationType) bool {
	env := getEnvVerify()
	return env == string(envVerifyValueAll) || env == strings.ToLower(string(verification))
}

// EnableVerifications sets `envVerify` and returns a function that
// can be used to bring the original settings.
func EnableVerifications(verification VerificationType) func() {
	previousEnv := getEnvVerify()
	os.Setenv(envVerify, string(verification))
	return func() {