Skip to content

v3: multiple improvements on test_helpers package [pool_helper.go] #485

@bigbes

Description

@bigbes

The test_helpers/pool_helper.go file contains utility functions used across
integration and e2e tests to interact with Tarantool clusters. While
functional, the code can be significantly improved for type safety,
readability, maintainability, and consistency with modern Go and testing
practices.

Below are specific, actionable improvements to apply:


1. Use typed argument structs instead of interface{}

Replace generic interface{} parameters with explicit structs:

  • ProcessListenOnInstance(interface{})ProcessListenOnInstance(args ListenOnInstanceArgs)
  • CheckStatuses(interface{})CheckStatuses(args CheckStatusesArgs)

Define these structs in the same file (or a shared types.go if reused elsewhere):

type ListenOnInstanceArgs struct {
    ConnPool *tarantool.ConnectionPool
    Mode     tarantool.RequestMode
    // other fields as needed
}

type CheckStatusesArgs struct {
    Instances []InstanceSpec
    Expected  string
}

This improves clarity, enables IDE autocompletion, and prevents runtime panics from type mismatches.


2. Prefer switch over chained if/else for grouped validations

When multiple conditions are checked in sequence (e.g., after a function call), use a switch statement for better readability:

Before:

if err != nil {
    return fmt.Errorf("fail to Eval: %s", err.Error())
}
if len(data) < 1 {
    return fmt.Errorf("response.Data is empty after Eval")
}

After:

switch {
case err != nil:
    return fmt.Errorf("fail to Eval: %w", err)
case len(data) < 1:
    return errors.New("response.Data is empty after Eval")
}

Use errors.New or fmt.Errorf with %w for proper error wrapping.


3. Use typed decoding (GetTyped) instead of manual type assertions

Where possible, decode responses directly into typed variables using GetTyped to shift type safety to the client library and reduce boilerplate:

Before:

data, err := listenArgs.ConnPool.Do(req, listenArgs.Mode).Get()
if err != nil { /* ... */ }
port, ok := data[0].(string)
if !ok {
    return fmt.Errorf("response.Data is incorrect after Eval")
}
return port

After:

var out []string
err := listenArgs.ConnPool.Do(req, listenArgs.Mode).GetTyped(&out)
if err != nil {
    return fmt.Errorf("failed to decode response: %w", err)
}
if len(out) == 0 {
    return errors.New("empty response from Eval")
}
return out[0]

This eliminates fragile type assertions and leverages the library’s built-in decoding.


4. Avoid reflect.DeepEqual; use domain-specific comparators

Prefer custom comparison functions like the existing compareTuples over reflect.DeepEqual, which can be brittle (e.g., with unexported fields, pointer vs value differences, or map ordering). Use compareTuples or introduce new helpers like compareInstances where appropriate.


5. Replace custom Retry logic with testify/assert.Eventually

Instead of a hand-rolled retry loop, use the standard testing utility:

assert.Eventually(t, func() bool { return cond() }, 5*time.Second, 500*time.Millisecond)

This integrates cleanly with test output, respects test timeouts, and reduces duplication.


6. Use structured box API instead of raw CallRequest

Replace low-level calls like:

tarantool.NewCallRequest("box.info")

with the typed helper:

info, err := box.New(conn).Info()

This returns a structured *box.Info object, making field access safer and more readable (e.g., info.Status instead of parsing data[0].(map[interface{}]interface{})).


7. Fix SetInstanceRO.checkRole to return proper errors

The checkRole method currently uses panic or silent failures. Refactor it to return an error so callers can handle or wrap it appropriately.


8. Extract common “execute on all instances in parallel” logic

Both InsertOnInstances and SetClusterRO contain nearly identical code that:

  • Iterates over all instances
  • Executes an operation in parallel (via goroutines)
  • Collects and joins all errors

Extract this into a reusable helper, for example:

func ExecuteOnAll(dialers []tarantool.Dialer, fn func(tarantool.Dialer) error) error {
    var wg sync.WaitGroup
    var errs []error
    var mu sync.Mutex

    wg.Add(len(dialer))
    for _, dialer := range dialers {

        go func(i tarantool.Dialer) {
            defer wg.Done()
            if err := fn(i); err != nil {
                mu.Lock()
                defer mu.Unlock()

                errs = append(errs, fmt.Errorf("instance %s: %w", i.Name, err))
            }
        }(dialer)
    }

    wg.Wait()
    if len(errs) > 0 {
        return errors.Join(errs...)
    }
    return nil
}

Then simplify InsertOnInstances and SetClusterRO to use this helper.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions