Skip to content

rewrite mapreduce upsert tests with async/await#9176

Open
emilygilberts wants to merge 1 commit intoapache:masterfrom
emilygilberts:refactor-unit-test-mapreduce
Open

rewrite mapreduce upsert tests with async/await#9176
emilygilberts wants to merge 1 commit intoapache:masterfrom
emilygilberts:refactor-unit-test-mapreduce

Conversation

@emilygilberts
Copy link
Contributor

  • rewrote the unit tests for mapreduce.js-upsert using async/await
    (left the mapreduce.js-utils tests as they are, because they are more readable & straightforward with the current syntax)

Copy link
Member

@janl janl left a comment

Choose a reason for hiding this comment

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

this looks good overall, I’m just wondering if it would make sense to DRY out the get and put functions a little bit, it’s three lines each when the only difference is the what the promise resolves with.

Spitballing here:

instead of

   const res = await upsert(
      {
        get: function () {
          return Promise.reject({ status: 404 });
        },
        put: function () {
          return Promise.resolve({ rev: 'abc' });
        },
      },
      'foo',
      function () {
        return { difference: "something" };
      },
    );
function makeFun (action, value) {
  return function() { return Promise[action](value) };
}

// [...]

    const res = await upsert(
      {
        get: makeFun('resolve', {status: 404}),
        put: makeFun('reject', { rev: 'abc' }),
      },
      'foo',
      function () { return { difference: "something" };
      },
    );

Might be too fancy, feel free to shoot me down :)

it('should throw an error if the doc errors', async function () {
try {
await upsert(
{
Copy link
Member

Choose a reason for hiding this comment

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

just a style nit, but I’d be okay with having the opening curlies start with the method: upsert({ to save a mostly empty line

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.

2 participants

Comments