Windows interop

[richardschneider]

No description provided.

[codecov]

Codecov Report

Merging #154 into master will increase coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   90.48%   90.62%   +0.13%     
==========================================
  Files          14       14              
  Lines         736      736              
==========================================
+ Hits          666      667       +1     
+ Misses         70       69       -1

Impacted Files	Coverage Δ
src/index.js	79.85% <0%> (-2.16%)	⬇️
src/decision-engine/index.js	87.9% <0%> (-0.47%)	⬇️
src/types/wantlist/index.js	96.42% <0%> (+3.57%)	⬆️
src/want-manager/index.js	94.91% <0%> (+6.77%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab69719...5beb22c. Read the comment docs.

[richardschneider]

@diasdavid before merging, need PR libp2p/js-libp2p-kad-dht#20 to be released. Then update this package.json

[daviddias]

That is not a valid reason to skip a test. This Window PR should not be disabling tests to please the AppVeyor CI

[richardschneider]

This is not just pleasing appveyor. Both travis and circle fail with this test enabled.

[richardschneider]

looks like js-ipfs-repo is misbehaving. I'll investigate.

Error: version mismatch: expected v6, found v5
      at get (node_modules\ipfs-repo\src\version.js:57:27)
      at store.get (node_modules\ipfs-repo\src\version.js:31:9)
      at node_modules\graceful-fs\graceful-fs.js:78:16
      at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:513:3)

[Stebalien]

@richardschneider we recently bumped the repo version in go-ipfs and I believe that may have been done in js as well. @dryajov?

[richardschneider]

Thanks @Stebalien Many of the JS tests have a repo as a test fixture. does this mean we have to update all the test fixtures!

[Stebalien]

Not sure. Honestly, I know almost nothing about js-ipfs (I also don't really see the point in repo compatibility but that's a different issue).

[richardschneider]

See ipfs repo issue #152

[richardschneider]

@diasdavid LGTM please review

[dryajov]

@richardschneider I believe you are right, we might need to get the fixtures upgraded, but @diasdavid should be able to tell for sure.

[richardschneider]

@dryajov This would be a big task. I suggest that ipfs-repo should gracefully upgrade.

[daviddias]

@Stebalien we set ourselves to interop fully with go in as many points as possible so that a user would not have to think which IPFS implementation it is using. Unfortunately, go-ipfs does change the repo often and there are no specs or compliance tests we can use to keep it up and so we've separated paths for a while now. That to say we still keep the promise of trying to be compatible with go-ipfs as best we can.

In this case, all that was needed was just bumping the repo version. We use copies of go-ipfs created repos so that we can test our serializers and deserializers correctly.

[daviddias]

Don't really know what happened in one of the 4 jobs of appveyor

Seems that it is not an issue from this module. Merging :)

[richardschneider]

Yes, I've seem this error now and then. It only happens on a PR build. I was looking into this afternoon and got side tracked by repo version number.