-
Notifications
You must be signed in to change notification settings - Fork 27
Conversation
1e9d909
to
17ba126
Compare
17ba126
to
e21a5f0
Compare
@achingbrain let me know your thoughts. Specially, if we should keep the same interface testing behaviour of receiving the libp2p module instance for testing, or if we should change all the interface tests to receive the libp2p instance. This can also be a follow up PR to not break the consistency with the other interfaces now |
My gut feeling here is that this is not a good idea as it turns interface tests into integration tests which may hinder our ability to completely exercise the interface during the test, if some aspects of the interface require certain setup or other conditions. |
Unrelated but part of me thinks we should split the interface tests from the interfaces themselves, otherwise you end up with aegir being an undeclared dep of code that can be required when running in production. |
Yeah, I agree let's wait for testground for more complex integration tests. The PR as it is now, moves the pubsub subsystem tests from libp2p to the interface, which is how libp2p expects the routers to work and to have consistent behaviours. So, while they were technically used as integration tests in libp2p, they really are interface tests.
I have the same opinion. It is super weird to have the tests in the |
Context: libp2p/js-libp2p#857 let's remove peerDeps and get node16 in libp2p CI.
This basically grabs the pubsub subsystem tests from
js-libp2p
and put them hereThis PR restructures the pubsub tests to not use the routers as devDependencies in the libp2p repo, delegating the ownership of the integration testing to the modules. As a result, this PR moves the pubsub core tests from libp2p to the interface. This way, routers will run them as compliance tests with libp2p.
Discussion point/suggestion:
Unblocks: