transports/tcp: Unit test tokio feature

[mxinden]

As of today libp2p-tcp is only tested with the async-std feature. Make the unit tests test both the async-std and the tokio feature. Otherwise things like tokio-rs/tokio#3001 go unnoticed without testing the chat-tokioexample when upgrading (e.g. #1796).

[ericjmiller]

Is there a simple way to make TokioTcpConfig and TcpConfig structs swap based on feature?

I was thinking of doing this:

    fn config() -> Result<Transport, io::Error> {
        if cfg!(feature = "tokio") {
            Ok(TokioTcpConfig::new())
        } else if cfg!(feature = "async-std") {
            Ok(TcpConfig::new())
        } else {
            io::Error("No Feature set.")
        }
    }

But returns:
help: specify the associated types: 'Transport<Output = Type, Error = Type, Listener = Type, ListenerUpgrade = Type, Dial = Type>'

I'm hoping the return object can be used in all tests. Then you wouldn't even really need to specify the feature for the unit tests. Thoughts?

[mxinden]

@ericjmiller first of all, thanks for looking into this.

Cargo features have to be designed in an additive fashion. Say both the Tokio and the Async-std feature are enabled: In your example above, which transport should be returned?

I would suggest a similar approach to the one we follow in our mdns tests. Here we use a macro to template the unit tests, invoking that macro with the Tokio mdns service when the tokio feature is enabled and invoking the macro with the async std mdns service when the async-std feature is enabled. With both enabled, the macro is invoked twice.

What do you think @ericjmiller?

[ericjmiller]

Cargo features have to be designed in an additive fashion. Say both the Tokio and the Async-std feature are enabled: In your example above, which transport should be returned?

Ah, okay. Thanks.

regarding the mdns tests - that strategy makes sense to me. I'll shoot for that implementation. Stay tuned ...