ronantakizawa/codereview-bench huggingface dataset:
Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:
		return &ClientStream{
			Stream: &Stream{
				buf: &recvBuffer{
					c:  make(chan recvMsg),
					mu: sync.Mutex{},
				},
			},
			done:       make(chan struct{}),
			headerChan: make(chan struct{}),
		}
	}

	testClient := func(ts *ClientStream) *http2Client {
		return &http2Client{
			mu: sync.Mutex{},
			activeStreams: map[uint32]*ClientStream{
				0: ts,
			},
			controlBuf: newControlBuffer(make(<-chan struct{})),
		}
	}

	for _, test := range []struct {
		name            string
		metaHeaderFrame *http2.MetaHeadersFrame
		endStreamFlag   http2.Flags
		wantStatus      *status.Status
	}{
		{
			name: "1xx with END_STREAM is error",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			endStreamFlag: http2.FlagHeadersEndStream,
			wantStatus: status.New(
				codes.Internal,
				"protocol error: informational header with status code 100 must not have END_STREAM set",
			),
		},
		{
			name: "1xx without END_STREAM is ignored",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			endStreamFlag: 0,
			wantStatus:    nil,
		},

Review comment (suggestion):
This field should be renamed to something more general like `httpFlags`.  `EndStream` is one value for a flag, the field here can store any http flag value, not just end stream.

Code after:
		return &ClientStream{
			Stream: &Stream{
				buf: &recvBuffer{
					c:  make(chan recvMsg),
					mu: sync.Mutex{},
				},
			},
			done:       make(chan struct{}),
			headerChan: make(chan struct{}),
		}
	}

	testClient := func(ts *ClientStream) *http2Client {
		return &http2Client{
			mu: sync.Mutex{},
			activeStreams: map[uint32]*ClientStream{
				0: ts,
			},
			controlBuf: newControlBuffer(make(<-chan struct{})),
		}
	}

	for _, test := range []struct {
		name            string
		metaHeaderFrame *http2.MetaHeadersFrame
		httpFlags       http2.Flags
		wantStatus      *status.Status
	}{
		{
			name: "1xx with END_STREAM is error",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			httpFlags: http2.FlagHeadersEndStream,
			wantStatus: status.New(
				codes.Internal,
				"protocol error: informational header with status code 100 must not have END_STREAM set",
			),
		},
		{
			name: "1xx without END_STREAM is ignored",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			httpFlags:  0,
			wantStatus: nil,
		},
Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			endStreamFlag: 0,
			wantStatus:    nil,
		},
	} {
		t.Run(test.name, func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.endStreamFlag,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = \ngot: %v\nwant: %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}


Review comment (bug):
```go
t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
```
See https://google.github.io/styleguide/go/decisions#useful-test-failures

Code after:
				Fields: []hpack.HeaderField{
					{Name: ":status", Value: "100"},
				},
			},
			httpFlags:  0,
			wantStatus: nil,
		},
	} {
		t.Run(test.name, func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.httpFlags,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}
Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:
	} {

		t.Run(test.name, func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus
			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
			}
		})
		t.Run(fmt.Sprintf("%s-end_stream", test.name), func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    http2.FlagHeadersEndStream,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus
			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
			}
		})
	}
}

func TestConnectionError_Unwrap(t *testing.T) {
	err := connectionErrorf(false, os.ErrNotExist, "unwrap me")
	if !errors.Is(err, os.ErrNotExist) {
		t.Error("ConnectionError does not unwrap")
	}
}

// Test that in the event of a graceful client transport shutdown, i.e.,
// clientTransport.Close(), client sends a goaway to the server with the correct

Review comment (suggestion):
```suggestion
// or may not also be the initial header frame (trailers-only response).
```

Terminology from https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses

Code after:
		},
	}

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			cs := newTestClientStream()
			s := newTestHTTP2Client(cs)

			tc.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 1,
				},
			}

			s.operateHeaders(tc.metaHeaderFrame)
			got := cs.status
			want := tc.wantStatus
			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Errorf("operateHeaders(%v) got status %q, want %q", tc.metaHeaderFrame, got, want)
			}
		})
	}
}

// TestClientDecodeTrailer validates the handling of trailer frames, which may
// or may not also be the initial header frame (header-only response).
func (s) TestClientDecodeTrailer(t *testing.T) {
	tests := []struct {
		name                string
		metaHeaderFrame     *http2.MetaHeadersFrame
		wantEndStreamStatus *status.Status
	}{
		{
			name: "valid_trailer",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: "content-type", Value: "application/grpc"},
					{Name: "grpc-status", Value: "0"},
					{Name: ":status", Value: "200"},
				},
			},
			wantEndStreamStatus: status.New(codes.OK, ""),
		},
		{
			name: "missing_content_type_in_grpc_mode",
			metaHeaderFrame: &http2.MetaHeadersFrame{
				Fields: []hpack.HeaderField{
					{Name: "grpc-status", Value: "0"},
					{Name: ":status", Value: "200"},
				},
			},
Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:
		},
	} {
		t.Run(test.name, func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.httpFlags,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}

func TestDeleteStreamMetricsIncrementedOnlyOnce(t *testing.T) {
	// Enable channelz for metrics collection
	if !channelz.IsOn() {
		channelz.TurnOn()
	}
	defer internal.ChannelzTurnOffForTesting()

	// Create a test server with channelz support
	serverConfig := &ServerConfig{
		ChannelzParent: channelz.RegisterServer("test server"),
	}
	defer channelz.RemoveEntry(serverConfig.ChannelzParent.ID)

	server, client, cancel := setUpWithOptions(t, 0, serverConfig, normal, ConnectOptions{})
	defer cancel()
	defer server.stop()
	defer client.Close(fmt.Errorf("closed manually by test"))

	waitWhileTrue(t, func() (bool, error) {
		server.mu.Lock()
		defer server.mu.Unlock()
		if len(server.conns) == 0 {
			return true, fmt.Errorf("timed-out while waiting for connection to be created on the server")
		}
		return false, nil
	})

Review comment (bug):
We define tests as methods on the `grpctest.Tester` to run leak checks for goroutines, times and fail tests if errors are logged by gRPC. We should change this to be a method on the tester.
```go
func (s) TestDeleteStreamMetricsIncrementedOnlyOnce(t *testing.T) {
```

Code after:
			wantStatus: nil,
		},
	} {
		t.Run(test.name, func(t *testing.T) {
			ts := testStream()
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.httpFlags,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}

func (s) TestDeleteStreamMetricsIncrementedOnlyOnce(t *testing.T) {
	// Enable channelz for metrics collection
	defer internal.ChannelzTurnOffForTesting()
	if !channelz.IsOn() {
		channelz.TurnOn()
	}

	for _, test := range []struct {
		name                string
		eosReceived         bool
		wantStreamSucceeded int64
		wantStreamFailed    int64
	}{
		{
			name:                "StreamsSucceeded",
			eosReceived:         true,
			wantStreamSucceeded: 1,
			wantStreamFailed:    0,
		},
		{
			name:                "StreamsFailed",
			eosReceived:         false,
			wantStreamSucceeded: 0,
			wantStreamFailed:    1,
		},
Repository: grpc/grpc-go
File: internal/transport/transport_test.go (Go)

Code before:

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.httpFlags,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}

func TestDeleteStreamMetricsIncrementedOnlyOnce(t *testing.T) {
	// Enable channelz for metrics collection
	if !channelz.IsOn() {
		channelz.TurnOn()
	}
	defer internal.ChannelzTurnOffForTesting()

	// Create a test server with channelz support
	serverConfig := &ServerConfig{
		ChannelzParent: channelz.RegisterServer("test server"),
	}
	defer channelz.RemoveEntry(serverConfig.ChannelzParent.ID)

	server, client, cancel := setUpWithOptions(t, 0, serverConfig, normal, ConnectOptions{})
	defer cancel()
	defer server.stop()
	defer client.Close(fmt.Errorf("closed manually by test"))

	waitWhileTrue(t, func() (bool, error) {
		server.mu.Lock()
		defer server.mu.Unlock()
		if len(server.conns) == 0 {
			return true, fmt.Errorf("timed-out while waiting for connection to be created on the server")
		}
		return false, nil
	})

	// Get the server transport
	server.mu.Lock()
	var serverTransport *http2Server
	for st := range server.conns {

Review comment (refactor):
Can you move this `defer` statement into the above `if` block? This should ensure the the channelz state is the back to the same state as it was before the test started.

Code after:
			s := testClient(ts)

			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
				FrameHeader: http2.FrameHeader{
					StreamID: 0,
					Flags:    test.httpFlags,
				},
			}

			s.operateHeaders(test.metaHeaderFrame)

			got := ts.status
			want := test.wantStatus

			if got.Code() != want.Code() || got.Message() != want.Message() {
				t.Fatalf("operateHeaders(%v); status = %v, want %v", test.metaHeaderFrame, got, want)
			}
		})
	}
}

func (s) TestDeleteStreamMetricsIncrementedOnlyOnce(t *testing.T) {
	// Enable channelz for metrics collection
	defer internal.ChannelzTurnOffForTesting()
	if !channelz.IsOn() {
		channelz.TurnOn()
	}

	for _, test := range []struct {
		name                string
		eosReceived         bool
		wantStreamSucceeded int64
		wantStreamFailed    int64
	}{
		{
			name:                "StreamsSucceeded",
			eosReceived:         true,
			wantStreamSucceeded: 1,
			wantStreamFailed:    0,
		},
		{
			name:                "StreamsFailed",
			eosReceived:         false,
			wantStreamSucceeded: 0,
			wantStreamFailed:    1,
		},
	} {
		t.Run(test.name, func(t *testing.T) {
			ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
			defer cancel()
Repository: grpc/grpc-go
File: internal/xds/clients/xdsclient/ads_stream.go (Go)

Code before:
func (fc *adsFlowControl) setPending(pending bool) {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	if fc.stopped {
		return
	}

	fc.pending = pending
	if !pending {
		fc.cond.Broadcast()
	}
}

func (fc *adsFlowControl) runIfPending(f func()) bool {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	if fc.stopped {
		return false
	}

	// If there's a pending update, run the function while still holding the
	// lock. This ensures that the pending state does not change between the
	// check and the function call.
	pending := fc.pending
	if fc.pending {
		f()
	}
	return pending
}

// wait blocks until all the watchers have consumed the most recent update.
func (fc *adsFlowControl) wait() {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	for fc.pending && !fc.stopped {
		fc.cond.Wait()
	}
}


Review comment (bug):
Is there a reason to copy the variable here?
I suspected it's to shorten the code (i.e. use `pending` instead of `fc.pending`), but the next line uses `fc.pending`. The mutex is held, so it doesn't seem possible that `fc.pending` is updated during the execution of `f`, unless `f` is mutating the flow control object's state, which would risk inconsistent state or deadlocks if done through methods on `adsFlowControl`.

Code after:
	defer fc.mu.Unlock()

	fc.stopped = true
	fc.cond.Broadcast()
}

// setPending changes the internal state to indicate whether there is an update
// pending consumption by all watchers. If there is no longer a pending update,
// the condition variable is signaled to allow the recv method to proceed.
func (fc *adsFlowControl) setPending(pending bool) {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	if fc.stopped {
		return
	}

	fc.pending = pending
	if !pending {
		fc.cond.Broadcast()
	}
}

func (fc *adsFlowControl) runIfPending(f func()) bool {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	if fc.stopped {
		return false
	}

	// If there's a pending update, run the function while still holding the
	// lock. This ensures that the pending state does not change between the
	// check and the function call.
	if fc.pending {
		f()
	}
	return fc.pending
}

// wait blocks until all the watchers have consumed the most recent update.
// Returns true if the flow control was stopped while waiting, false otherwise.
func (fc *adsFlowControl) wait() bool {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	for fc.pending && !fc.stopped {
		fc.cond.Wait()
	}

	return fc.stopped
Repository: grpc/grpc-go
File: internal/xds/clients/xdsclient/ads_stream.go (Go)

Code before:

	if fc.stopped {
		return false
	}

	// If there's a pending update, run the function while still holding the
	// lock. This ensures that the pending state does not change between the
	// check and the function call.
	if fc.pending {
		f()
	}
	return fc.pending
}

// wait blocks until all the watchers have consumed the most recent update.
// Returns true if the flow control was stopped while waiting, false otherwise.
func (fc *adsFlowControl) wait() bool {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	for fc.pending && !fc.stopped {
		fc.cond.Wait()
	}

	return fc.stopped
}


Review comment (suggestion):
I think this should this should return `!fc.stopped` to keep the previous behaviour, or the check above `if !s.fc.wait() ` needs to be inverted. Previously `fc.wait()` was returning `false` when the context expired. Now, `fc.wait()` is returning true if the flow control object was stopped.

Code after:
	defer fc.mu.Unlock()

	if fc.stopped {
		return false
	}

	// If there's a pending update, run the function while still holding the
	// lock. This ensures that the pending state does not change between the
	// check and the function call.
	if fc.pending {
		f()
	}
	return fc.pending
}

// wait blocks until all the watchers have consumed the most recent update.
// Returns true if the flow control was stopped while waiting, false otherwise.
func (fc *adsFlowControl) wait() bool {
	fc.mu.Lock()
	defer fc.mu.Unlock()

	for fc.pending && !fc.stopped {
		fc.cond.Wait()
	}

	return fc.stopped
}
Repository: grpc/grpc-go
File: internal/transport/flowcontrol.go (Go)

Code before:

import (
	"fmt"
	"math"
	"sync"
	"sync/atomic"
)

// writeQuota is a soft limit on the amount of data a stream can
// schedule before some of it is written out.
type writeQuota struct {
	_ noCopy
	// get waits on read from when quota goes less than or equal to zero.
	// replenish writes on it when quota goes positive again.
	ch chan struct{}
	// done is triggered in error case.
	done <-chan struct{}
	// replenish is called by loopyWriter to give quota back to.
	// It is implemented as a field so that it can be updated
	// by tests.
	replenish func(n int)
	quota     int32
}

func initWriteQuota(wq *writeQuota, sz int32, done <-chan struct{}) {
	*wq = writeQuota{
		quota: sz,
		ch:    make(chan struct{}, 1),
		done:  done,
	}
	wq.replenish = wq.realReplenish
}

func (w *writeQuota) get(sz int32) error {
	for {
		if atomic.LoadInt32(&w.quota) > 0 {
			atomic.AddInt32(&w.quota, -sz)
			return nil
		}
		select {
		case <-w.ch:
			continue
		case <-w.done:
			return errStreamDone
		}
	}
}

func (w *writeQuota) realReplenish(n int) {
	sz := int32(n)
	newQuota := atomic.AddInt32(&w.quota, sz)

Review comment (suggestion):
This syntax does feel a little weird to me. Did you try some of these options to see if they read better (and don't perform worse)?
- directly set fields of `wq` in here instead of setting `wq` to a completely new instance of `writeQuota`
- Can this `initWriteQuota` be a method on `Stream`?
- Can we make the zero value of `writeQouta` something that can actually work?

This comment applies to other types as well like `recvBuffer`.

Thanks.

Code after:

import (
	"fmt"
	"math"
	"sync"
	"sync/atomic"
)

// writeQuota is a soft limit on the amount of data a stream can
// schedule before some of it is written out.
type writeQuota struct {
	_ noCopy
	// get waits on read from when quota goes less than or equal to zero.
	// replenish writes on it when quota goes positive again.
	ch chan struct{}
	// done is triggered in error case.
	done <-chan struct{}
	// replenish is called by loopyWriter to give quota back to.
	// It is implemented as a field so that it can be updated
	// by tests.
	replenish func(n int)
	quota     int32
}

// init allows a writeQuota to be initialized in-place, which is useful for
// resetting a buffer or for avoiding a heap allocation when the buffer is
// embedded in another struct.
func (w *writeQuota) init(sz int32, done <-chan struct{}) {
	w.quota = sz
	w.ch = make(chan struct{}, 1)
	w.done = done
	w.replenish = w.realReplenish
}

func (w *writeQuota) get(sz int32) error {
	for {
		if atomic.LoadInt32(&w.quota) > 0 {
			atomic.AddInt32(&w.quota, -sz)
			return nil
		}
		select {
		case <-w.ch:
			continue
		case <-w.done:
			return errStreamDone
		}
	}
}

func (w *writeQuota) realReplenish(n int) {
	sz := int32(n)
Repository: grpc/grpc-go
File: balancer/randomsubsetting/randomsubsetting_test.go (Go)

Code before:
import (
	"context"
	"encoding/json"
	"fmt"
	"math"
	"strings"
	"testing"
	"time"

	"github.com/google/go-cmp/cmp"
	"google.golang.org/grpc"
	"google.golang.org/grpc/credentials/insecure"
	"google.golang.org/grpc/internal"
	"google.golang.org/grpc/internal/grpctest"
	"google.golang.org/grpc/internal/stubserver"
	"google.golang.org/grpc/peer"
	"google.golang.org/grpc/resolver"
	"google.golang.org/grpc/resolver/manual"
	"google.golang.org/grpc/serviceconfig"

	iserviceconfig "google.golang.org/grpc/internal/serviceconfig"
	testgrpc "google.golang.org/grpc/interop/grpc_testing"
	testpb "google.golang.org/grpc/interop/grpc_testing"
)

var defaultTestTimeout = 120 * time.Second

type s struct {
	grpctest.Tester
}

func Test(t *testing.T) {
	grpctest.RunSubTests(t, s{})
}

func (s) TestParseConfig(t *testing.T) {
	parser := bb{}
	tests := []struct {
		name    string
		input   string
		wantCfg serviceconfig.LoadBalancingConfig
		wantErr string
	}{
		{
			name:    "invalid-json",
			input:   "{{invalidjson{{",
			wantErr: "invalid character",
		},
		{
			name:    "empty_config",
			input:   `{}`,

Review comment (suggestion):
We cannot have a test run for 2 minutes. Please change this to something like `5s` or `10s` which are the more commonly used values in our repo.

Code after:
	"context"
	"encoding/json"
	"fmt"
	"strings"
	"testing"
	"time"

	xxhash "github.com/cespare/xxhash/v2"
	"github.com/google/go-cmp/cmp"
	"google.golang.org/grpc/balancer"
	"google.golang.org/grpc/internal/balancer/stub"
	"google.golang.org/grpc/internal/grpctest"
	iserviceconfig "google.golang.org/grpc/internal/serviceconfig"
	"google.golang.org/grpc/internal/testutils"
	"google.golang.org/grpc/resolver"
	"google.golang.org/grpc/serviceconfig"

	_ "google.golang.org/grpc/balancer/roundrobin" // For round_robin LB policy in tests
)

var defaultTestTimeout = 5 * time.Second

type s struct {
	grpctest.Tester
}

func Test(t *testing.T) {
	grpctest.RunSubTests(t, s{})
}

func (s) TestParseConfig(t *testing.T) {
	parser := bb{}
	tests := []struct {
		name    string
		input   string
		wantCfg serviceconfig.LoadBalancingConfig
		wantErr string
	}{
		{
			name:    "invalid_json",
			input:   "{{invalidjson{{",
			wantErr: "json.Unmarshal failed for configuration",
		},
		{
			name:    "empty_config",
			input:   `{}`,
			wantErr: "SubsetSize must be greater than 0",
		},
		{
			name:    "subset_size_zero",
			input:   `{ "subsetSize": 0 }`,