feat: dynamic format loading #178
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| /* eslint-env mocha */ | ||
| 'use strict' | ||
|
|
||
| const chai = require('chai') | ||
| const dirtyChai = require('dirty-chai') | ||
| const expect = chai.expect | ||
| chai.use(dirtyChai) | ||
| const BlockService = require('ipfs-block-service') | ||
| const dagCBOR = require('ipld-dag-cbor') | ||
|
|
||
| const IPLDResolver = require('../src') | ||
|
|
||
| module.exports = (repo) => { | ||
|
There was a problem hiding this comment. Would it make sense to add a test that a format is not dynamically loaded when it was already statically added? |
||
| describe('IPLD format support', () => { | ||
| let data, cid | ||
|
|
||
| before((done) => { | ||
| const bs = new BlockService(repo) | ||
| const resolver = new IPLDResolver({ blockService: bs }) | ||
|
|
||
| data = { now: Date.now() } | ||
|
There was a problem hiding this comment. Why the |
||
|
|
||
| dagCBOR.util.cid(data, (err, c) => { | ||
| expect(err).to.not.exist() | ||
| cid = c | ||
| resolver.put(data, { cid }, done) | ||
| }) | ||
| }) | ||
|
|
||
| describe('Dynamic format loading', () => { | ||
| it('should fail to dynamically load format', (done) => { | ||
| const bs = new BlockService(repo) | ||
| const resolver = new IPLDResolver({ | ||
| blockService: bs, | ||
| formats: [] | ||
| }) | ||
|
|
||
| resolver.get(cid, '/', (err) => { | ||
| expect(err).to.exist() | ||
| expect(err.message).to.equal('No resolver found for codec "dag-cbor"') | ||
| done() | ||
| }) | ||
| }) | ||
|
|
||
| it('should fail to dynamically load format via loadFormat option', (done) => { | ||
| const errMsg = 'BOOM' + Date.now() | ||
|
There was a problem hiding this comment. Why the There was a problem hiding this comment. In this case I want to ensure the error that was thrown was the error that I threw. I'm asserting on the error message so I need a value that is not going to cause a false positive with any other error that could be thrown. So, yes, I guess I could just use a static string but there's a tiny bit more risk of it causing a false possitive in the future than a value that is randomly generated. In general I know that tests are not isolated from side effects, so I like to know my tests are dealing with their own unique data. As an example we share block stores across multiple tests and they don't clean up after themselves. By the way, I don't believe they should clean up and we can't guarantee that it'll happen (in the case of failures) or that unintentional side effects don't happen. Ideally we'd use a new block store for every test, but I appreciate that's not always practical....The point is, using randomly generated data is one way of better isolating tests from side effects caused by other tests. A completely contrived example: I'm testing I can put a value with IPLD and I use a static value. Many weeks later someone adds a test before mine that puts that same value in the block store. Since the block store is shared, when my test runs the code somehow determines the value is already in the store and returns success. Same result, but it's a different code path that's being tested now, and maybe that's masking a error with the code path that writes to disk... There was a problem hiding this comment. BTW, I'm happy to change if you disagree or would prefer! There was a problem hiding this comment. Thanks for the explanation. It makes sense, happy to keep it that way. |
||
| const bs = new BlockService(repo) | ||
| const resolver = new IPLDResolver({ | ||
| blockService: bs, | ||
| formats: [], | ||
| loadFormat (codec, callback) { | ||
| if (codec !== 'dag-cbor') return callback(new Error('unexpected codec')) | ||
| setTimeout(() => callback(new Error(errMsg))) | ||
| } | ||
| }) | ||
|
|
||
| resolver.get(cid, '/', (err) => { | ||
| expect(err).to.exist() | ||
| expect(err.message).to.equal(errMsg) | ||
| done() | ||
| }) | ||
| }) | ||
|
|
||
| it('should dynamically load missing format', (done) => { | ||
| const bs = new BlockService(repo) | ||
| const resolver = new IPLDResolver({ | ||
| blockService: bs, | ||
| formats: [], | ||
| loadFormat (codec, callback) { | ||
| if (codec !== 'dag-cbor') return callback(new Error('unexpected codec')) | ||
| setTimeout(() => callback(null, dagCBOR)) | ||
| } | ||
| }) | ||
|
|
||
| resolver.get(cid, '/', (err, result) => { | ||
| expect(err).to.not.exist() | ||
| expect(result.value).to.eql(data) | ||
| done() | ||
| }) | ||
| }) | ||
|
|
||
| it('should not dynamically load format added statically', (done) => { | ||
| const bs = new BlockService(repo) | ||
| const resolver = new IPLDResolver({ | ||
| blockService: bs, | ||
| formats: [dagCBOR], | ||
| loadFormat (codec) { | ||
| throw new Error(`unexpected load format ${codec}`) | ||
| } | ||
| }) | ||
|
|
||
| resolver.get(cid, '/', (err, result) => { | ||
| expect(err).to.not.exist() | ||
| expect(result.value).to.eql(data) | ||
| done() | ||
| }) | ||
| }) | ||
| }) | ||
| }) | ||
| } | ||
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.
Wow, I didn't know that this syntax is a things. It's nice to learn new things by code reviewing.