Improve integration tests for public API

[gavv]

Currently we have 3 integrations tests for our C API:

• without FEC
• with Reed-Solomon FEC and without losses
• with Reed-Solomon FEC and with losses

We need to cover more parameter combinations:

• Use single context for sender and receiver, or use separate contexts.
• Use one sender per receiver, or connect multiple senders to receiver (sequential version).
• Use one sender per receiver, or connect multiple senders to receiver (parallel version).
• Use one receiver, or create multiple receivers sharing one context, and connect a sender to each receiver.
• Use Reed-Solomon or LDPC-Staircase FEC schemes, with and without losses.
• Enable or disable packet interleaving.
• Use different resampler profiles.
• Use different clock sources (external or internal).
• Test a few different presets of FEC block size, latency, and breakage detection parameters. For example, one low latency preset and one high latency preset.

Before adding more tests, it would be also nice to extract Context, Sender, Receiver, and Proxy from test_sender_receiver.cpp into separate files in src/tests/roc_library (i.e. test_context.h, test_sender.h, and so on).

This work can be split into multiple PRs, one or a few tests per PR.

[CleverShovel]

I would like to try write these tests.

[gavv]

@CleverShovel Great, you're welcome!

[CleverShovel]

@gavv Is this normal that in roc_library directory there are files like test_context.cpp, test_sender.cpp and others and I will add test_context.h and others but they will not related to these cpp?

[gavv]

Good catch. Since this issue was created, the file layout in tests was changed a bit. According to the new layout, these files should be named like roc_library/test_helpers/context.h, roc_library/test_helpers/sender.h, etc. See, for example, tests for roc_pipeline or roc_audio.

[CleverShovel]

@gavv Does each item from the list in issue description mean concrete tests or only a parameter to combine?

[gavv]

Well, it's just a list of features to be covered somehow. Obviously, we can't create a test for every possible combination of these features because there were too much tests. So, likely, we should create one or a few test for every specific item. E.g., for Enable or disable packet interleaving we can create one test covering both cases or two separate tests, one w/o interleaving, one w/ interleaving. That's not a strict requirement, though. For example, if we already have another test covering transfer w/o interleaving, there is no need to duplicate it.

[CleverShovel]

@gavv As far as I can see for parallel test of multiple senders and one receiver I should add another one roc_endpoint and roc_receiver in Receiver class and additionally adjust while cycle in method Receiver::run(). Am I right?

[gavv]

You can connect multiple senders to the same endpoint(s). The receiver will mix them. You need to adjust the received stream checking to expect the stream mixed from multiple senders.

The hard thing here is that you don't known how the streams from different senders are shifted. One possible way to deal with it is to make the first sender produce stream like 10, 20, 30, ..., and the second sender 1, 2, 3, ... Then you'll read from the receiver something like 25, 36, 47, ... and from that you can deduce the original sequences of both senders and check them separately.

[gavv]

multiple_senders_one_receiver_sequential is failing quite often, I disabled it for now.

https://travis-ci.org/github/gavv/roc/jobs/671349254

https://travis-ci.org/github/gavv/roc/jobs/671349250

@CleverShovel Could you please take a look?

[CleverShovel]

@gavv ok, I'll check test more carefully

[CleverShovel]

@gavv When I debugged the test in Receiver::run() I got these data in rx_buff:


Receiver expect data in range (0, 1) but there can be very small numbers as well as very big numbers and nan. Can I add some function that check that received number in range (0, 1) ?
Also I think that variable wait_for_signal not very useful here. One time the tests were failed because of this variable. When in prev_sample number from the previous frame and wait_for_signal = true value of prev_sample isn't set to beginning of current frame. Unfortunately reproduce this error I can't.

[gavv]

Receiver expect data in range (0, 1) but there can be very small numbers as well as very big numbers and nan.

This looks very strange. If receiver produces samples out of range [0; 1] [-1; 1], it's a bug. But are you sure that you're debugging it correctly? How did you get the numbers on screenshot?

Can I add some function that check that received number in range (0, 1) ?

Yes, adding such a check is legit and makes sense. But the range is [-1; 1].

Also I think that variable wait_for_signal not very useful here.

IIRC it was added for reason. Maybe @alexandremgo can provide some details on this.

[gavv]

I think the problem with multiple_senders_one_receiver_sequential is that we're not waiting until the receiver will fully process the samples from the first sender before starting the second one.

I guess we should change the flow to the following:

• run first sender
• wait until receiver starts producing all zeros for some period of time (we could add something like receiver.wait_at_least_n_zeros())
• run second sender

[gavv]

BTW, note that this test likely will work incorrectly under a debugger because of its real-time nature. (Though receiver should not produce numbers out of [-1; 1] range anyway).

[gavv]

An interesting detail: both failures on travis were on sample #3999.

[gavv]

@CleverShovel Hi, do you have any further plans on this?

[CleverShovel]

@gavv Sorry for long delay, I'm was busy. I try to find time for this by the end of this month.

[gavv]

Great!

[gavv]

Unassigning so that someone can continue the work during hacktoberfest. @CleverShovel feel free to let me know if you want to be re-assigned though.

[gavv]

I've created separate issues for remaining tests:

• Add integration tests for concurrently running senders #557
• Add integration tests for more protocol combinations #558
• Add integration tests for external clock #559
• Add integration tests for resampler profiles #560