test(ping): add interface-core-api ping tests and fix impl

[alanshaw]

depends on: ipfs-inactive/interface-js-ipfs-core#273

closes #762

[daviddias]

LGTM.

Does this also close #762 and #725?

[alanshaw]

ooh definitely #762 and hopefully #725 🤞

[daviddias]

CI is failing.

  1) .ping .ping sends the specified number of packets:

     Uncaught AssertionError: expected [Error: Peer lookup error: failed to find any peer in table] to not exist

      at ipfsdA.ping (node_modules/interface-ipfs-core/js/src/ping.js:51:30)

      at pump (src/ping.js:49:18)

      at node_modules/pump/index.js:75:7

      at f (node_modules/once/once.js:25:25)

      at Writable.<anonymous> (node_modules/pump/index.js:33:5)

      at Writable.f (node_modules/once/once.js:25:25)

      at Writable.onfinish (node_modules/end-of-stream/index.js:30:27)

      at finishMaybe (node_modules/readable-stream/lib/_stream_writable.js:630:14)

      at afterWrite (node_modules/readable-stream/lib/_stream_writable.js:492:3)

      at _combinedTickCallback (internal/process/next_tick.js:144:20)

      at process._tickCallback (internal/process/next_tick.js:180:9)

[alanshaw]

Looking into it. I found #770 which I think is causing a lot of flake.

My current plan is to wait until the nodes are connected in the before function before starting the tests.

[daviddias]

Good catch @alanshaw 👏🏽

There is still a race condition on the pubsub test

  1) .pubsub multiple nodes connected light-load tests call subscribe/unsubscribe 10 times:


      Uncaught AssertionError: expected [ Array(1) ] to deeply equal []

      + expected - actual


      -[

      -  "pubsub-tests-95d85efa861b4f19e5391bc4457d2e86"

      -]

      +[]

      

      at ipfs1.pubsub.ls (node_modules/interface-ipfs-core/js/src/pubsub.js:666:39)

      at stringlistToArray (src/utils/stringlist-to-array.js:6:3)

      at send (src/utils/send-request.js:208:7)

      at f (node_modules/once/once.js:25:25)

      at streamToValue (src/utils/stream-to-json-value.js:30:5)

      at concat (src/utils/stream-to-value.js:12:22)

      at ConcatStream.<anonymous> (node_modules/concat-stream/index.js:37:43)

      at finishMaybe (node_modules/readable-stream/lib/_stream_writable.js:630:14)

      at afterWrite (node_modules/readable-stream/lib/_stream_writable.js:492:3)

      at _combinedTickCallback (internal/process/next_tick.js:144:11)

      at process._tickCallback (internal/process/next_tick.js:180:9)

[alanshaw]

I've been working on that.

I've been experimenting and have changed the unsubscribe logic to only callback on the last event I can listen for on a dropped connection. This still does not guarantee that by the time pubsub.ls is called that the topic won't appear in the list.

The only way I can get this to work is to add an artificial timeout. I don't know enough go to figure out if there's some sort of delay when a connection is dropped before a subscription is unsubscribed. It could be that, or just that go simply doesn't find out about the dropped connection before the call to pubsub.ls is made.

I don't know if this is also a problem when talking to a js-ipfs daemon via the api. I will check.

[alanshaw]

Just to follow up, it appears that js-ipfs doesn't need the artificial timeout so it's leaning towards a go-ipfs issue right now.

[daviddias]

@alanshaw seems you submitted multiple small PR that all fix a few of the flakyness on tests. Do you want to go and create a master PR to test all of them together?

[alanshaw]

I was going to get the small ones merged and then rebase this - hope thats ok 😛

[daviddias]

You can now update this to 0.4.15 and also update ipfsd-ctl :)

[alanshaw]

@diasdavid I think this is looking good to go now. Tests are passing on macOS and linux more reliably than before.

Windows tests are consistently failing. I tracked that back to this commit where you upgraded go-ipfs to 0.4.14.

I've branched off this branch and reverted to go-ipfs-dep@0.4.13 and whilst the tests still fail, the swarm connectivity issues are gone (and the failing tests all look like things we changed since 0.4.14). You can inspect the CI logs for that experiment here.

I think this might be a regression in go-ipfs...

[daviddias]

@Stebalien see comment above #768 (comment)

[daviddias]

@alanshaw mind reporting that as a potential bug to go-ipfs?

Also, can you report as a bug the situation where a ipfs.pubsub.unsubscribe followed by an ipfs.pubsub.ls will not present an up to date list of subscriptions (race condition)? //cc @Stebalien

[alanshaw]

Will do

[alanshaw]

Long time coming but issue for swarm connectivity issues: ipfs/kubo#5146