Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

race on jsipfs init #796

Merged
merged 1 commit into from
Mar 17, 2017
Merged

race on jsipfs init #796

merged 1 commit into from
Mar 17, 2017

Conversation

daviddias
Copy link
Member

sometimes, travis fails with a repo that wasn't fully initiated onjsipfs init (just CLI)

I'm being unable to replicate it locally, so this PR is just me throwing things to see if I can figure it out :)

@daviddias daviddias added the status/in-progress In progress label Mar 17, 2017
@daviddias
Copy link
Member Author

daviddias commented Mar 17, 2017

Node.js stable passes on circle, but fails on travis. Also passes on my machine.

err.code = 'ENOENT'
cb(err)
})
this.node.once('start', cb)
Copy link
Member

Choose a reason for hiding this comment

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

need to wrap the cb into a once otherwise there could be a double calling

Copy link
Member Author

Choose a reason for hiding this comment

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

but there is a once listener, so it only does it once

Copy link
Member

Choose a reason for hiding this comment

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

but you register it twice, once for start and once for error

Copy link
Member Author

Choose a reason for hiding this comment

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

they are mutually exclusive, but I'll add it anyway just to be sure 👍

'Access-Control-Expose-Headers',
'X-Stream-Output, X-Chunked-Output, X-Content-Length')

this.server.start((err) => {
Copy link
Member

Choose a reason for hiding this comment

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

This can be just this.server.start(cb) and then the rest below be in the next block


this.server.start((err) => {
if (err) {
return callback(err)
Copy link
Member

Choose a reason for hiding this comment

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

this should be cb

// for the CLI to know the where abouts of the API
fs.writeFileSync(apiFilePath, api.info.ma)

callback()
Copy link
Member

Choose a reason for hiding this comment

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

this should be cb

@@ -10,14 +10,15 @@ describe('daemon', () => {
let ipfs

beforeEach(() => {
repoPath = '/tmp/ipfs-test-' + Math.random().toString().substring(2, 8)
repoPath = '/tmp/ipfs-test-not-found-' + Math.random().toString().substring(2, 8)
Copy link
Member

Choose a reason for hiding this comment

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

why not-found?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was just making it obvious for my tests :)

@daviddias
Copy link
Member Author

before I squashed, this was passing:
image

Now it is just hogged because travis is not even starting the tests. Going to go ahead and merge :)

@daviddias daviddias merged commit 044162a into master Mar 17, 2017
@daviddias daviddias deleted the fix/cli-nit branch March 17, 2017 16:57
@daviddias daviddias removed the status/in-progress In progress label Mar 17, 2017
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.

2 participants