-
-
Notifications
You must be signed in to change notification settings - Fork 571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use ipfsd-ctl to create test IPFS instances #335
Conversation
a166fe9
to
e394205
Compare
test/replicate.test.js
Outdated
if(orbitdb1) | ||
await orbitdb1.stop() | ||
Object.keys(testAPIs).forEach(API => { | ||
describe.only(`orbit-db - Replication (${API})`, function() { |
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.
The .only
here will avoid other tests from running on CI (Just pointing it out to avoid it being merged accidentally)
I'm getting an error log while running tests locally – this time with the automatic replication suite but it happened with the replication suite before as well. Maybe it's just a negative test failing as expected?
|
5109479
to
d5c1ff9
Compare
The tests are finally passing again. I added a workaround for the "Error: underlying socket has been closed" error, as it seems really weird. See here https://github.com/orbitdb/orbit-db/pull/335/files#diff-11b1d30de414c2bebf3a03b5f2bd1b73R48 and here https://github.com/orbitdb/orbit-db/pull/335/files#diff-b1d58ced77879cec2a109423fb3755a8R87. Not sure what's happening but it may not be related to libp2p because if we comment out the leveldb's method to destroy the files from the local filesystem (https://github.com/orbitdb/orbit-db-cache/blob/master/Cache.js#L59 which gets called from https://github.com/orbitdb/orbit-db-store/blob/master/src/Store.js#L171), the error is gone 😕❓❓❓😕 Otherwise, this is ready for merge but would love another round of review to catch any issues! |
d5c1ff9
to
230dc66
Compare
I have no idea :/ Pulled your last changes, reinstalled everything and now tests are passing. Previously it was failing intermittently, so it does look like a race condition situation |
343fbfb
to
e445331
Compare
Found the bug and it was indeed a race condition. We were not waiting for unsubscribing from pubsub internally and that caused the race (receiving messages after closing the database). Fixed it and updated the PR and removed the timeout workarounds from the test. Ready to merge. |
Fix default database creation tests Add more test utils Remove obsolete public methods from OrbitDB Workaround for "underlying socket has been closed" error in replication test Update package-lock
e445331
to
3318b6a
Compare
Thanks @thiagodelgado111 and everyone else who tested and reviewed this PR! ❤️ It's now in master and would love some more testing. Will be merging #328 and then make a new release (0.20.0). Hopefully this week. |
This PR brings back ipfsd-ctl to create test IPFS instances.
In addition, this PR will move a lot of the boilerplate test setup code to it's on test utils as well as fix some basic tests for the stores and remove couple of obsolete/unused public methods from OrbitDB class.