roc-streaming / roc-go

Golang bindings for Roc Toolkit.
https://roc-streaming.org
MIT License
22 stars 10 forks source link

[41]:... unit tests for sender and receiver errors #65

Closed Darrellbor closed 1 year ago

Darrellbor commented 1 year ago

PR for #41 Implemented

Darrellbor commented 1 year ago

@gavv @ortex PR ready for review

coveralls commented 1 year ago

Coverage Status

Coverage: 85.822% (+8.7%) from 77.127% when pulling 8952369090505f0735cd845b90f208338fdedc99 on Darrellbor:41-unit_tests_for_sender_and_receiver_errors into ee4158d3a0370defea1197a6008293c8d764a2a0 on roc-streaming:main.

gavv commented 1 year ago

Also @Asalle

ortex commented 1 year ago

Nice work!

a couple of tests could be added:

also, point for discussion (@gavv ):

Maybe add test Receiver_Closed (or something like that) and remove receiverClosedBefore scenario from all tests? Because we need to check that each function call (except Сlose()) on a closed receiver return an error e.g:


func TestReceiver_Closed(t *testing.T) {
    tests := []struct {
        name      string
        operation func(receiver *Receiver) error
    }{
        {
            name: "SetMulticastGroup after close",
            operation: func(receiver *Receiver) error {
                return receiver.SetMulticastGroup(SlotDefault, InterfaceAudioSource, "127.0.0.1")
            },
        },
        {
            name: "Bind after close",
            operation: func(receiver *Receiver) error {
                return receiver.Bind(SlotDefault, InterfaceAudioSource, nil)
            },
        },
        {
            name: "ReadFloats after close",
            operation: func(receiver *Receiver) error {
                recFloats := make([]float32, 2)
                return receiver.ReadFloats(recFloats)
            },
        },
    }
    for _, tt := range tests {
        ctx, err := OpenContext(ContextConfig{})
        require.NoError(t, err)
        require.NotNil(t, ctx)

        receiver, err := OpenReceiver(ctx, makeReceiverConfig())
        require.NoError(t, err)
        require.NotNil(t, receiver)

        err = receiver.Close()
        require.NoError(t, err)

        require.Equal(t, errors.New("receiver is closed"), tt.operation(receiver))

        err = ctx.Close()
        require.NoError(t, err)
    }
}
gavv commented 1 year ago

I agree with Andrey, separating this check will make the table tests more uniform, which is the point of tables. Ideally table tests should have minimal amount of conditions and if tests have rather different flow, they should belong to different tables / tests.

Darrellbor commented 1 year ago

@gavv @ortex reviewed based on the comments and ready for re-review

gavv commented 1 year ago

Thanks!