-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
BREAKING CHANGE: uses new multiaddr, libp2p-interfaces, etc
data: message, | ||
topicIDs: [topic] | ||
})) | ||
|
||
expect(messageToEmit).to.eql(expected) | ||
}) | ||
|
||
it.skip('does not send received message back to original sender', async () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, just writing it. I pushed an update to see if CI was healthier with the libp2p rc, it is!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added now.
floodsub1 = new Floodsub(peer1, defOptions) | ||
floodsub2 = new Floodsub(peer2, defOptions) | ||
const libp2p = { | ||
peerId: await PeerId.create(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to not use createPeer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spinning up libp2p nodes is overkill for unit tests. Ideally we wouldn't depend on libp2p at all in this module as libp2p itself depends on floodsub creating a loop which we really need to break in order to simplify publishing new versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
BREAKING CHANGE: uses new multiaddr, libp2p-interfaces, etc