Skip to content

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

Merged
gavv merged 8 commits into
roc-streaming:mainfrom
Darrellbor:41-unit_tests_for_sender_and_receiver_errors
Mar 28, 2023
Merged

[41]:... unit tests for sender and receiver errors#65
gavv merged 8 commits into
roc-streaming:mainfrom
Darrellbor:41-unit_tests_for_sender_and_receiver_errors

Conversation

@Darrellbor

Copy link
Copy Markdown
Contributor

PR for #41
Implemented

  • TestReceiver_Bind
  • TestReceiver_ReadFloats
  • TestSender_Connect
  • TestSender_WriteFloats

@Darrellbor

Copy link
Copy Markdown
Contributor Author

@gavv @ortex PR ready for review

@coveralls

coveralls commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

Coverage Status

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

@gavv

gavv commented Mar 25, 2023

Copy link
Copy Markdown
Member

Also @Asalle

@ortex

ortex commented Mar 27, 2023

Copy link
Copy Markdown
Member

Nice work!

a couple of tests could be added:

  • for Sender.Connect, Receiver.Bind: add tests with an invalid endpoint that fails to convert to C endpoint, e.g. with invalid protocol Protocol(1)
  • for Sender.Writefloat, Receiver.ReadFloats: add tests with zero-sized frame []float32{}

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

gavv commented Mar 27, 2023

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Contributor Author

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

Comment thread roc/sender_test.go
Comment thread roc/sender_test.go Outdated
@Darrellbor Darrellbor requested a review from ortex March 28, 2023 07:44
@gavv gavv added the status: ready for review Pull request can be reviewed label Mar 28, 2023
@gavv gavv merged commit fca50ff into roc-streaming:main Mar 28, 2023
@gavv

gavv commented Mar 28, 2023

Copy link
Copy Markdown
Member

Thanks!

@Darrellbor Darrellbor deleted the 41-unit_tests_for_sender_and_receiver_errors branch March 28, 2023 13:43
@gavv gavv removed the status: ready for review Pull request can be reviewed label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants