Less skipped tests #494
Less skipped tests #494
Conversation
| @@ -60,14 +60,15 @@ module.exports = (ctl) => { | |||
| it('.get updatedConfig', (done) => { | |||
| ctl.config.get((err, config) => { | |||
| expect(err).not.to.exist | |||
| expect(config).to.deep.equal(updatedConfig()) | |||
| console.log(config, updatedConfig()) | |||
There was a problem hiding this comment.
Is a console.log in the tests? Serves no purpose, seems to have been leftover from debugging.
There was a problem hiding this comment.
It was a joke, that's why I wrote the ;). Already removed
There was a problem hiding this comment.
Ah, sorry, went way above my head 😿
There was a problem hiding this comment.
No worries 😸 jokes are somtimes hard to understand on the internetz
| expect(res.result.version).to.equal(pkgversion) | ||
| expect(res.result).to.have.a.property('commit') | ||
| expect(res.result).to.have.a.property('repo') | ||
| expect(res.result).to.have.a.property('Version', pkgversion) |
There was a problem hiding this comment.
Should not actually check the version rather than just making sure something is there?
There was a problem hiding this comment.
Suggestion would be to read version from somewhere, then assert with that, rather than hard coding the version
There was a problem hiding this comment.
that's exactly what is happening here, pkgversion is read from package.json which is the version that js-ipfs uses as baseline when answering the version command.
There was a problem hiding this comment.
Oh, crap, you're right, as always. I'll never get used to Chai/Expect syntax...
| } | ||
|
|
||
| console.log(`js-ipfs version: ${version}`) | ||
| console.log(`js-ipfs version: ${version.version}`) |
There was a problem hiding this comment.
Should probably use the logger that I've created before, instead of using console.log
There was a problem hiding this comment.
Let's open an issue for that, and do the conversion, if we do it at once, rather than here and there.
| console.log(JSON.stringify(value, null, 2)) | ||
| } else { | ||
| console.log(value) | ||
| } |
There was a problem hiding this comment.
Same here as below, we should probably try using the logger rather than console.log
There was a problem hiding this comment.
we can make that in a single PR and change all of them, no?
dignifiedquire
force-pushed
the
fix/less-skipping
branch
from
42bb55b to
be6f679
Compare
|
5 🌟 |
Made by @achingbrain for ipfs-inactive/js-ipfs-http-client#957 License: MIT Signed-off-by: Alan Shaw <[email protected]>
I found some tests that were skipped and resulted in actual broken code. This is fixed now and things are more http-api and interface-ipfs-core compliant than before :)