Skip to content
This repository has been archived by the owner on Aug 1, 2023. It is now read-only.

test: Pin API interop #19

Merged
merged 4 commits into from
Jul 12, 2018
Merged

test: Pin API interop #19

merged 4 commits into from
Jul 12, 2018

Conversation

JonKrone
Copy link
Contributor

@JonKrone JonKrone commented Apr 4, 2018

js-ipfs is close to finishing work on the pin command: ipfs/js-ipfs#1045.

This suite of tests should give us some confidence that the go and js implementations are compatible and that, to the extent that go's pins are, js-ipfs' are correct.

Open for review 😃

@ghost ghost assigned JonKrone Apr 4, 2018
@ghost ghost added the status/in-progress In progress label Apr 4, 2018
@JonKrone JonKrone mentioned this pull request Apr 4, 2018
2 tasks
@JonKrone
Copy link
Contributor Author

JonKrone commented Apr 5, 2018

Everything is ✔️ with the current pin work on js-ipfs and the latest release of go-ipfs.

I am working on testing with the current master of go-ipfs but am working through build issues :).

@dryajov jenkins CI is failing on some pubsub ascii tests on linux/windows and also on the exchange-files tests on windows. Are any of those new? Can I do something to help there?

@daviddias daviddias changed the title Feat/pin test: Pin API interop May 29, 2018
@JonKrone
Copy link
Contributor Author

Confirming that these tests pass with the current master of go-ipfs and the pin-api branch of js-ipfs.

This is a rough draft of a suite of tests to verify that the go and js ipfs implementations behave the same when adding/removing pins to the datastore. I've found that they produce essentially the same pinsets. There are a few secondary differences I've noticed such as: fresh go repos include the readme files as pinned while the js impl does not. Draft #2 coming up! Lots of code to dedupe
@JonKrone
Copy link
Contributor Author

JonKrone commented Jun 1, 2018

Only failing tests are related to: #23

Note: This currently uses "ipfs": "github:ipfs/js-ipfs#pin-api" to enable tests, will need to update it here after it's merged into js-ipfs's master.

@ghost ghost assigned daviddias Jul 5, 2018
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and the tests pass against js-ipfs master!

// tests that js' pin.flush and go's pin.load are compatible
it.skip('js -> go', function () {
// skipped because go can not be spawned on a js repo due to changes in
// the DataStore config [link]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to understand this better...can [link] be a real boy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go-ipfs now has all sorts of DataStore mumbo jumbo because of their badger datastore and because we don't create it nor use it go-ipfs complains saying that our repo config is not good.

We can add the datastore mumbo jumbo, I just got tired of playing catch with go-ipfs changing repo config all the time (5x ++) and not updating the spec nor telling anyone. That said, yes, interop + specs are awesome and we should strive to have them.

package.json Outdated
"hat": "0.0.3",
"ipfs": "~0.29.0",
"ipfs": "github:ipfs/js-ipfs",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N.B. Needs to be updated after 0.30 release

@daviddias
Copy link
Member

Only failure was PubSub on Windows
image

Not related to this PR.

@daviddias daviddias merged commit d89fd06 into master Jul 12, 2018
@ghost ghost removed the status/in-progress In progress label Jul 12, 2018
@daviddias daviddias deleted the feat/pin branch July 12, 2018 23:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants