Skip to content

Conversation

@Joezo
Copy link
Contributor

@Joezo Joezo commented Mar 11, 2019

What does this PR do?

Adds a test for the memcache package.

Related issues

closes #93

Checklist

  • I have checked the contributing document
  • I have added or updated any relevant documentation
  • I have added or updated any relevant tests

@jackdclark jackdclark self-requested a review March 11, 2019 15:58
@Joezo Joezo added the graphql-hooks-memcache graphql-hooks-memcache package label Mar 11, 2019
Copy link
Contributor

@jackdclark jackdclark left a comment

Choose a reason for hiding this comment

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

Minor things, otherwise LGTM 👍

it('lists all keys', () => {
cache.set('foo', 'bar')
cache.set('bar', 'baz')
expect(cache.keys().length).toEqual(2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe assert the return value instead of or as well as the length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what the keys are though as their generated so can't assert on them.

@Joezo Joezo merged commit bc3854e into master Mar 11, 2019
@Joezo Joezo deleted the memcache-tests branch March 11, 2019 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

graphql-hooks-memcache graphql-hooks-memcache package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test coverage for graphql-hooks-memcache

2 participants