Closed alexjg closed 1 year ago
If you're looking to review the test changes specifically I would recommend reading 22acec2. I did the rename in a separate commit so this commit actually has a useful diff - unlike the diff view of the whole PR which presents the test files as just deleted and reinserted.
Thanks! Will review today.
Thanks for the detailed review. I think I've addressed or replied to everything.
I'm working on updating this codebase to be interoperable with the 1.0 of the JS implementation. As I do this I need to write a bunch more tests. I felt that the existing tests were a little verbose (as is often the case with the first version of a test suite where you're focusing on getting the tests written at all) and so before I wrote more tests I thought I would take the time to refactor things a little bit.
There are a few main changes:
new_remote_repo
to accept theSink
andStream
as separate arguments. This makes calling this function easier for cases where the sink and stream are not the same object (such as in channels), which leads us into the next point ...Network
network adapter with a simpler thing which is really just a wrapper around atokio::sync::mpsc::channel(1)
. You can find this intests::network::tincans
tests/network
module so that they all compile in one go and can reuse the utilities intincans
async fn
usingtokio::test
. This means less incidental complexity juggling runtimes and channels to communicate the results of spawned tasks back to the main testString
toNetworkError
so it's easier to debug when it occurs and also add a bunch moretracing
poll_close
various sinks