Skip to content
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

Running the tests in the browser #26

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Conversation

fbaiodias
Copy link
Member

Hey!

I'm trying to get the repo running on a browser, but I'm having some trouble doing it, so a review would be appreciated :)

I started by specifying two different entry points on package.json, one for now which uses fs-blob-store as before and one for the browser which uses idb-blob-store.

I've also started setting up karma for running the tests, however they're currently very slow and inconsistent, having different results on different runs, but I haven't really understood why.

Please let me know if you find anything

@daviddias
Copy link
Member

Let's finish this up today after ArcticJS talks :)

@daviddias daviddias changed the title Browser compatibility (WIP) Running the tests in the browser Jan 21, 2016
}).end(file.value, cb)
}, () => {
// FIXME debuging possible idb-blob-store bug
setTimeout(done, 1000)
Copy link
Member

Choose a reason for hiding this comment

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

This line was added for testing purposes.

It seems that the end event on the returned write streams is emitted before it actually finishes, making the testes fail, since the db isn't consistent.

Copy link
Member

Choose a reason for hiding this comment

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

Is this still required with local-storage-blob-store?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, just removed the setTimeout

repo.datastore.createReadStream(buf)
.pipe(bl((err, data) => {
expect(err).to.not.exist
const eq = fs.readFileSync(process.cwd() + '/tests/test-repo/blocks/12207028/122070286b9afa6620a66f715c7020d68af3d10e1a497971629c07606bfdb812303d.data').equals(data)
Copy link
Member

Choose a reason for hiding this comment

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

Can you brfs this file so that the test also works in the browser?

@fbaiodias fbaiodias force-pushed the browser branch 3 times, most recently from 9149447 to cbc223d Compare January 21, 2016 19:15
singleRun: true,

// List plugins explicitly, since autoloading karma-webpack
// won't work here
Copy link
Member

Choose a reason for hiding this comment

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

why? this should work if they are listed in package.json

Copy link
Member Author

Choose a reason for hiding this comment

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

first time using karma, just removed that :)

@dignifiedquire
Copy link
Member

I can try running it later. please let me know if you made progress so I don't do things twice

@daviddias
Copy link
Member

@xicombd, can you check @dignifiedquire comments and adjust to those?

@dignifiedquire
Copy link
Member

Cleared all my confusions and running well on my machine, just left some notes about things that could be improved

@fbaiodias
Copy link
Member Author

@xicombd, can you check @dignifiedquire comments and adjust to those?

Yes, on it right now.

@dignifiedquire: everything is working now, the problems I was having were apparently related to idb-blob-store racing conditions, so I ended up implementing local-storage-blob-store which seems to be doing it's job.

@daviddias
Copy link
Member

🎉🎉🎉🎉
:D

Thank you for finishing this @xicombd :)

daviddias added a commit that referenced this pull request Jan 21, 2016
Running the tests in the browser
@daviddias daviddias merged commit e24b7f6 into ipfs:master Jan 21, 2016
@fbaiodias fbaiodias deleted the browser branch January 21, 2016 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants