Closed dshil closed 8 months ago
:umbrella: The latest upstream change (presumably these) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream change (presumably these) made this pull request unmergeable. Please resolve the merge conflicts.
That was huge!
I've rebased PR and fixed several minor issues (mostly mechanical), now going to merge it.
What I changed: (75641bf065e360a5fc58c2db4039cb9e8b14a0cd)
Added more TODO comments and reformatted them as TODO(gh-183): text
.
Reverted change in ReceiverEndpoint: use ReceiverSessionGroup instead of IWriter as before. On one hand, polymorphism is not really needed there (except tests, but see below). On the other hand, pipeline has quite complicated layout, and it's very convenient to use static typing to be able to see call graph from code.
Pruned SenderEndpoint and ReceiverEndpoint tests a bit:
First, I think it's not the best idea to add endpoint-level tests for how packets are going (I mean tests write_read_packet, pull_packets_failed). We already test packet handling in GREAT detail in source/sink-level tests, which cover endpoints as well (route_packets, pull_packets are covered). In source/sink tests, we already have a lot of facilities to generate mock input and to validate output (e.g. to produce valid and corrupt RTP packets), and I'd like to avoid duplicating it in endpoint-tests (which will happen if we'll start covering SenderEndpoint and ReceiverEndpoint at endpoint level). So I kept endpoint tests for construction, but dropped tests for packet flow.
Second, I also simplified no-memory test. I think there is no need to check that some protocols can be used even when arena fails (I don't consider it a feature, just a coincidence), so I excluded them from tests and tests became much cleaner.
In test::Proxy from public_api tests, there is no need to forward statuses, we just need to check that they're always OK - so I've simplified it.
Renamed some long test names (I'm a bit allergic).
GH-303