Tests for closing context

[Darrellbor]

PR for #54 implemented

• over arching test for context closing
• sub tests to handle cases like
  1. when there are no senders and no receivers
  2. when there are receivers
  3. when there are senders
  4. when there are senders and receivers

[Darrellbor]

@gavv PR ready for review

[ortex]

Thanks for PR!

Could you please also add a check that context closes successfully after closing the attached receiver/sender for these cases?

when there are receivers when there are senders when there are senders and receivers

[Darrellbor]

@ortex I've been looking into that, I just pushed the closing methods. they race condition test still fails locally

[Darrellbor]

@ortex never mind, I figured out the issue that was causing the race condition.

[coveralls]

Coverage: 74.847% (+0.4%) from 74.438% when pulling 285c2806a9e94f592425d486d487c89191f50014 on Darrellbor:54-test_context_closing into e2ef7a20276d4e1f13d3c58828add96ba3fcf03f on roc-streaming:main.

[ortex]

Now tests actually don't assert that context closing successfully after closing the attached receiver or sender.

And about the endpoint test - I think you need to update roc locally. No need to comment test. The related fix was released recently https://github.com/roc-streaming/roc-toolkit/releases/tag/v0.2.3

[ortex]

still not asserting success context closing for when there are senders and receivers case :)

Maybe delete defer func() for receiver and sender and add something like that to the end?

    if tt.hasReceivers || tt.hasSenders {
        if tt.hasReceivers {
            err = receiver.Close()
            require.NoError(t, err)
        }
        if tt.hasSenders {
            err = sender.Close()
            require.NoError(t, err)
        }

        err = ctx.Close()
        require.NoError(t, err)
    }

[ortex]

need to revert https://github.com/roc-streaming/roc-go/pull/58/commits/c1834d21c193d7fbd51bd2c8dcfe32879a0a8cfe everything else LGTM

[Darrellbor]

@ortex this block still causes the test to fail both on my branch and on main

[ortex]

which block? If you are about endpoint_test you need to update roc itself (locally), not roc-go.

As you can see this test is enabled in main branch and CI is green: https://github.com/roc-streaming/roc-go/actions/runs/4409507925

[Darrellbor]

resolved

[gavv]

Thanks, looks good to me as well.

[gavv]

Cosmetic fix: 005ae7ef1d919f8398147d7f0f53e70aaa46e258