Skip to content

Commit

Permalink
feat(gatsby): update cache.set to resolve with set value (#11327)
Browse files Browse the repository at this point in the history
<!--
  Have any questions? Check out the contributing docs at https://gatsby.app/contribute, or
  ask in this Pull Request and a Gatsby maintainer will be happy to help :)
-->

## Description

<!-- Write a brief description of the changes introduced by this PR -->

Cache set method will now resolve to the stored value.

Also, I have never written Jest tests so this was a pretty exciting learning experience. Mocking was not very intuitive, especially due to the resolve function being in the callback of the get and set cache manager package. We also don't actually talk to the cache so the get tests feel a little wonky. Also, I do not know the shape of the error object sent from the cache manager or what actually can cause it so I mocked its existence with `!undefined` which is obviously just `true` but I wanted it to read as something that was defined and not just true. Is there a better/conventional way to say `err` is defined? Would `{}` be a better solution? Interested if anyone has a better testing strategy than just forcing the error argument to exist or not.

## Related Issues

<!--
  Link to the issue that is fixed by this PR (if there is one)
  e.g. Fixes #1234, Addresses #1234, Related to #1234, etc.
-->

Addresses #11275
  • Loading branch information
mgmolisani authored and wardpeet committed Feb 15, 2019
1 parent f7a5a35 commit 930164a
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 5 deletions.
47 changes: 44 additions & 3 deletions packages/gatsby/src/utils/__tests__/cache.js
@@ -1,10 +1,17 @@
const mockErrorValue = jest.fn()
const mockResultValue = jest.fn()

jest.mock(`cache-manager`, () => {
return {
caching: jest.fn(),
multiCaching: jest.fn().mockImplementation(() => {
multiCaching: jest.fn(() => {
return {
get: jest.fn(),
set: jest.fn(),
get: jest.fn((key, callback) => {
callback(mockErrorValue(), mockResultValue())
}),
set: jest.fn((key, value, args, callback) => {
callback(mockErrorValue())
}),
}
}),
}
Expand Down Expand Up @@ -98,4 +105,38 @@ describe(`cache`, () => {
containsThenMethod(cache.set(`a`, `b`))
})
})

describe(`set`, () => {
it(`resolves to the value it cached`, () => {
const cache = getCache()

return expect(cache.set(`a`, `b`)).resolves.toBe(`b`)
})

it(`resolves to undefined on caching error`, () => {
const cache = getCache()

mockErrorValue.mockReturnValueOnce(true)

return expect(cache.set(`a`, `b`)).resolves.toBeUndefined()
})
})

describe(`get`, () => {
it(`resolves to the found value`, () => {
const cache = getCache()

mockResultValue.mockReturnValueOnce(`result`)

return expect(cache.get()).resolves.toBe(`result`)
})

it(`resolves to undefined on caching error`, () => {
const cache = getCache()

mockErrorValue.mockReturnValueOnce(true)

return expect(cache.get()).resolves.toBeUndefined()
})
})
})
8 changes: 6 additions & 2 deletions packages/gatsby/src/utils/cache.js
Expand Up @@ -40,13 +40,17 @@ class Cache {

get(key) {
return new Promise(resolve => {
this.cache.get(key, (_, res) => resolve(res))
this.cache.get(key, (err, res) => {
resolve(err ? undefined : res)
})
})
}

set(key, value, args = {}) {
return new Promise(resolve => {
this.cache.set(key, value, args, (_, res) => resolve(res))
this.cache.set(key, value, args, err => {
resolve(err ? undefined : value)
})
})
}
}
Expand Down

0 comments on commit 930164a

Please sign in to comment.