Add unit tests for endpoint errors

[dhanusaputra]

close #42

it's not easy to set arguments where the test failed in the not covered errors so use mock func instead any guidance welcome 🙏

[gavv]

Thanks for PR!

You're right, some branches can't be covered without mocks, but apparently these branches are just impossible. For instance, if roc_endpoint_set_uri did not return error, then fromC will never fail.

I think it's OK to keep these impossible cases uncovered, and avoid usage of mocks at all. In these tests, it's much more interesting to cover combination of go bindings + C lib, not just bindings alone, because most bugs will happen when they start working together.

We can still cover a few possible cases without need for mocks:

• roc_endpoint_set_protocol will fail if proto does not belong to enum
• roc_endpoint_set_host will fail if host is empty
• roc_endpoint_set_port will fail if port is out of range [0; 65535] and is not -1
• roc_endpoint_set_resource will fail if resource has invalid format (see below)
• roc_endpoint_get_uri will fail if proto does not have default port, but port is -1 (e.g. RTP)
• roc_endpoint_get_uri will fail if proto does not support resource part, but resource is set (e.g. RTP)

Here is a part from URI ragel grammar (endpoint_uri_parse.rl from roc-toolkit):

        proto = ( [a-z0-9+]+ ) >start_token %set_proto;

        host = ('[' [^/@\[\]]+ ']' | [^/:@\[\]]+) >start_token %set_host;
        port = (digit+) >start_token %set_port;

        pchar = [^?#];

        path = ('/' pchar*) >start_token %set_path;
        query = (pchar*) >start_token %set_query;

        uri = ( proto '://' host (':' port)? )? path? ('?' query)?;

Here, path? ('?' query)? corresponds to resource. You can use this grammar to have an idea how to compose invalid resource.

[dhanusaputra]

thanks for your help! did some changes based on your guidance but only able to apply 2, please see below

• roc_endpoint_set_protocol will fail if proto does not belong to enum -> this branch already covered by other test with empty uri
• roc_endpoint_set_host will fail if host is empty -> the host cannot be empty since we checked endp.Host != ""
• roc_endpoint_set_port will fail if port is out of range [0; 65535] and is not -1 -> add 1 test case
• roc_endpoint_set_resource will fail if resource has invalid format (see below) -> add 1 test case
• roc_endpoint_get_uri will fail if proto does not have default port, but port is -1 (e.g. RTP) -> this branch already covered by other test case
• roc_endpoint_get_uri will fail if proto does not support resource part, but resource is set (e.g. RTP) -> this branch already covered by other test case

[coveralls]

Coverage: 73.784% (+0.8%) from 72.939% when pulling c0cf7890cc36cdeda066c59385ac35071b0e0d23 on dhanusaputra:unit-test-endpoint-errors into c445824928cf569915e65d5a55c8062976feaad1 on roc-streaming:main.

[gavv]

Great, thanks for your analysis.

Could you also add a few more cases please?

• [x] when protocol field != 0 but does not belong to enum (currently we cover only zero protocol)
• [ ] when port is zero (should work without error)
• [x] when port is negative but not -1 (currently we cover only -1 and > 0)
• [x] when both path and query are present (should work without error)

Probably these cases won't increase reported coverage, but they're still important.

roc_endpoint_get_uri will fail if proto does not have default port, but port is -1 (e.g. RTP) -> this branch already covered by other test case

roc_endpoint_get_uri will fail if proto does not support resource part, but resource is set (e.g. RTP) -> this branch already covered by other test case

All cases in TestEndpoint use RTSP, which supports default port and resource part. Let's add two cases which try to use RTP (not RTSP) with:

• [x] default port (-1) and
• [x] non-empty resource part.

[gavv]

I just got a thought, it would be also nice to add one successful case per each protocol (see here) to be sure that all constants in bindings are correctly mapped to corresponding C values.

(Currently we cover only RTSP).

[gavv]

Thank you!

I also tried to add this test:

when port is zero (should work without error)

And realized that there is a bug, created an issue: https://github.com/roc-streaming/roc-toolkit/issues/519

Here is a small follow-up PR: https://github.com/roc-streaming/roc-go/pull/56