Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add async guide #282

Merged
merged 14 commits into from
Dec 26, 2017
Merged

Add async guide #282

merged 14 commits into from
Dec 26, 2017

Conversation

lmiller1990
Copy link
Member

As discussed in #246 , some more guides for common pitfalls might be nice.

@eddyerburgh this is just my first draft, I'd like some input on the content, style, etc. Thanks!

@lmiller1990 lmiller1990 mentioned this pull request Dec 20, 2017
13 tasks
Copy link
Member

@eddyerburgh eddyerburgh left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! So I wasn't aware that watchers weren't running during update!

I'm going to run watchers as part of update, so your original example will pass.

We should keep the section on testing async APIs.

it looks like a good guide. Some minor points:

  • can you stylize methods without the parentheses, so done() becomes done.
  • we should mention update

wrapper.find('input').trigger('input')

wrapper.vm.$nextTick(() => {
expect(wrapper.vm.showButton).toBe(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this assertion throws, will the error be caught, or will the test time out? In my experience, it's the latter.

Copy link
Contributor

@callumacrae callumacrae Dec 20, 2017

Choose a reason for hiding this comment

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

Mind you, I've been using mocha which doesn't handle uncaught promise errors. might be different with jest

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it will time out @callumacrae . What is the best thing to do in this situation?

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure. i made an issue about it here: #213

@@ -0,0 +1,159 @@
# Testing Asynchronous Components
Copy link
Member

Choose a reason for hiding this comment

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

This headline might be confused with "Async components".

Maybe: "Testing Asynchronous Behaviour" ?

Copy link
Member Author

@lmiller1990 lmiller1990 Dec 20, 2017

Choose a reason for hiding this comment

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

Good catch @LinusBorg . I think your suggested title is better. Minor, but In my day to day life I spell it behaviour, should use use American English (no u) or English/Europe/everywhere else English?

Copy link
Member

Choose a reason for hiding this comment

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

Hm. I think we should probably stick do American english as that's the bigger "market"?

OTOH @eddyerburgh is british so ..., :-P

Copy link
Member

Choose a reason for hiding this comment

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

Indeed I am British 🇬🇧

Copy link
Member Author

Choose a reason for hiding this comment

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

I am Australian (English colony) so we use the same style. But I agree we should use the American English since I believe the official docs would use that.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should use US English

@lmiller1990
Copy link
Member Author

@eddyerburgh good idea about mentioning update. What exactly does update do, just force nextTick?

@eddyerburgh
Copy link
Member

@lmiller1990 At the moment, it forces a component re-render.

I think we should also make it run all the component and child component watchers. I'm trying to think whether this would have unintended consequences 🤔

@lmiller1990
Copy link
Member Author

Removed watch example, since that'll be solved in #283 .

Will look add adding more about update. Can you think of any good examples where running update is important for async tests @eddyerburgh ? I haven't had to use it much. Or were you just thinking to add something so users are more aware of how vue-test-utils works, in a synchronous fashion?

@eddyerburgh
Copy link
Member

In fact, calling a synchronous method that updates state will also require calling update.
Any time state is changed, you need to call update. In all vue-test-utils methods that change or can change state, like trigger, setData, setProps, update is called to trigger a re render.

If state is changed outside of vue-test-utils, by mutating the state directly, or by triggering a method, update must be called to update the render tree.

@lmiller1990 An example for this guide would be when you trigger an async method by calling it on the vm:

const Component = {
  template: `<div>{{someData}}</div>`,
  data() {
    return {
      someData: 0
    }
  },
  methods: {
    asyncMethod() {
     Promise.resolve(() => {
      this.someData++
     })
    }
  }
}

const wrapper = mount(Component)
wrapper.vm.asyncMethod()
await flushPromises()
wrapper.update()
expect(wrapper.text()).toBe(1)

@lmiller1990
Copy link
Member Author

@eddyerburgh I haven't actually ran into this issue - calling the methods on the wrapper seems to actually implicitly call update as well. The following (basically what you wrote above) actually passes.

<template>
  <div>
    {{someData}}
  </div>
</template>

<script>
  export default {
    data () {
      return {
        someData: 0
      }
    },
    methods: {
      asyncMethod () {
        Promise.resolve(() => {
        }).then(() => {
          this.someData += 1
        })
      }
    }
  }
</script>
import {shallow} from 'vue-test-utils'
import Update from './Update'
import flushPromises from 'flush-promises'

describe('Update', () => {
  it('works', async () => {
    const wrapper = shallow(Update)
    wrapper.vm.asyncMethod()
    await flushPromises()
    expect(wrapper.vm.someData).toBe(1)
    expect(wrapper.text()).toBe('1')
  })
})

I haven't actually hit any situations where I've had to call update. Do you have another example?

@eddyerburgh
Copy link
Member

Ah—when flush-promises has finished, the component will have re rendered.

Looks like we don't need to include updates in this guide 😂

@lmiller1990
Copy link
Member Author

@eddyerburgh ok. Is there anything else you think I should change or improve? Would be nice to get this in.

Also, I fixed up the failing tests regarding no trailing whitespace, but it appears to be actually linting the example code, and complaining that await is a reserved keyword and it also doesn't like @click. Any way to ignore these?

@wtho
Copy link
Contributor

wtho commented Dec 23, 2017

Just a thought:.
What if the asyncMethod is not promise-based, but e. g. callback-based? I know it is less often and maybe bad style, but it should still be possible to test this code.

</template>

<script>
import axios from 'axios'
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a line break after the import statement

}
</script>
```
A test can be written like this:
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a link to the Jest manual mocks page—https://facebook.github.io/jest/docs/en/manual-mocks.html#content

})
```

This test currently fails, because the assertion is called before the promise resolves. One solution is to use the npm package, `flush-promises`. which immediately resolve any unresolved promises. This test is also asynchronous, so like the previous example, we need to let the test runner know to wait before making any assertions.
Copy link
Member

@eddyerburgh eddyerburgh Dec 24, 2017

Choose a reason for hiding this comment

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

which immediately resolve any unresolved promises

to

which can be used to flush all pending resolved promise handlers

jest.mock('axios')

test('Foo', () => {
it('fetches async when a button is clicked', () => {
Copy link
Member

@eddyerburgh eddyerburgh Dec 24, 2017

Choose a reason for hiding this comment

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

You need to make the function async, that's why there's a linting error:
it('fetches async when a button is clicked', () => {

to

it('fetches async when a button is clicked', async () => {


This test currently fails, because the assertion is called before the promise resolves. One solution is to use the npm package, `flush-promises`. which immediately resolve any unresolved promises. This test is also asynchronous, so like the previous example, we need to let the test runner know to wait before making any assertions.

If you are using Jest, there are a few options, such as passing a `done` callback, as shown above. Another is to prepend the test with the ES7 'async' keyword. We can now use the the ES7 `await` keyword with `flushPromises`, to immediately resolve the API call.
Copy link
Member

Choose a reason for hiding this comment

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

such as passing a done callback, as shown above

It isn't shown above. Can you add an example using done. Instead of using flushPromises you can just use a setTimeout:

test('Foo', (done) => {
   it('fetches async when a button is clicked', () => {
     const wrapper = shallow(Foo)
     wrapper.find('button').trigger('click')
     setTimeout(() => {
          expect(wrapper.vm.value).toEqual('value')
          done()
     })
   })
 })

(note I haven't tested this code)

We should add a note on why this works, with a link to this blog post—https://jakearchibald.com/2015/tasks-microtasks-queues-and-schedules/. Basically, the microtask queue, which processes promise callbacks, runs before the task queue, which processes timeout callbacks. So when a setTimeout callback is executed, any resolved promises will have been executed.

Copy link
Member

@eddyerburgh eddyerburgh left a comment

Choose a reason for hiding this comment

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

Looks good, there's just a couple issues to resolve.

Can you test that the examples work, just so we're sure.

@lmiller1990
Copy link
Member Author

lmiller1990 commented Dec 24, 2017

Thanks for the feedback @eddyerburgh .

Updates:

  1. Add example using done
  2. Include the option to use $nextTick or setTimeout (both seem fine, I ran the tests. Do you have a preference? I used $nextTick in the example code, but mention both in the explanation.
  3. Include link about why setTimeout works. I didn't know why, that was a really interesting read.
  4. Improve some wording.
  5. Run all tests locally (was doing so anyway, but just to check).

PS merry christmas!

})
```

The reason `$nextTick` or `setTimeout` allow the test to pass is because the microtask queue where promise callbacks are processed run efore the task queue, where `$nextTick` and `setTimeout` are processed. This means by the time the `$nexTick` and `setTimeout` run, any promise callbacks on the microtask queue will have been executed. See [here](https://jakearchibald.com/2015/tasks-microtasks-queues-and-schedules/) for a more detailed explanation.
Copy link
Member

Choose a reason for hiding this comment

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

small typo — efore


If you are using Jest, there are a few options, such as passing a `done` callback, as shown above. Another is to prepend the test with the ES7 'async' keyword. We can now use the the ES7 `await` keyword with `flushPromises`, to immediately resolve the API call.
Another solution is to use the npm package, `flush-promises`, which can be used to flush all pending resolved promise handlers, and prepend the test with the ES7 'async' keyword. The ES7 `await` keyword with `flushPromises` can now be used to immediately resolve the API call.
Copy link
Member

Choose a reason for hiding this comment

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

Can we reword this:

Another solution is to use the npm package, flush-promises, which can be used to flush all pending resolved promise handlers, and prepend the test with the ES7 'async' keyword. The ES7 await keyword with flushPromises can now be used to immediately resolve the API call.

To something like this:

Another solution is to use an async function and the npm package flush-promises. flush-promises flushes all pending resolved promise handlers. You can await the call of flushPromises to flush the promises and improve the readability of your test.

@lmiller1990
Copy link
Member Author

lmiller1990 commented Dec 25, 2017

Done! Thanks for the feedback.

@doun
Copy link

doun commented Dec 25, 2017

@lmiller1990 Would u add an example for catching exceptions:
try{ await a(); expect(CONDITION); await b(); expect(CONDITION); } catch(e){ }
be careful that exception throw by expect() should NOT be caught, or it will pass all tests

@lmiller1990
Copy link
Member Author

lmiller1990 commented Dec 25, 2017

@doun the current test shouldn't swallow any exceptions, but throw an error. I haven't experienced what you described, can you link to an example/post some sample code? Or do you mean I should add an example showing how to test if an error was throw from some async code?

I think a small note might be worth including.

@doun
Copy link

doun commented Dec 25, 2017

It's not about current sample code.
I just did this days ago, sample code:

test("expect fail", async ()=>{
try{
  var resultA = await TaskA()
  expect(resultA).toBe(true)
  var resultB = await TaskB()
  expect(true).toBe(false)
}
//all exceptions caught, the test will pass
catch(e){
}
})

I wanted to catch only async exceptions, but the test framework(jest) also use exceptions to catch expect failures.

@lmiller1990
Copy link
Member Author

lmiller1990 commented Dec 25, 2017

@doun Thanks for the sample. It appears you are just testing regular JavaScript methods, not in the context of a Vue component? Or can you provide the component and some more context? What is TaskA and TaskB?

I'm not sure this belongs here -- of course it is important, but it doesn't feel specific enough to Vue components to include in this particular guide.

One good practise can be to use expect.assertions at the top of your tests - that way, if an error occurs and Jest skips to the catch block, because expect.assertions is not the correct amount, the test will fail.

@doun
Copy link

doun commented Dec 25, 2017 via email

Copy link
Member

@eddyerburgh eddyerburgh left a comment

Choose a reason for hiding this comment

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

Two final edits I'd like you to make, then we can merge 🙂

})
```

This test currently fails because the assertion is called before the promise in `fetchResults` resolves. Most unit test libraries provide a callback to let the runner know when the test is complete. Jest and Karma both use `done`. We can use `done` in combination with `$nextTick` or `setTimeout` to ensure any promises resolve before the assertion is made.
Copy link
Member

Choose a reason for hiding this comment

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

Jest and Karma both use

should be

Jest and Mocha both use

@@ -0,0 +1,98 @@
# Testing Asynchronous Behavior

To simplify testing, `vue-test-utils` applies updates _synchronously_. However, there are some techniques you need to be aware of when testing a component with asynchronous behavior such as callbacks or promises.
Copy link
Member

Choose a reason for hiding this comment

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

applies updates synchronously

to

applies DOM updates synchronously

@lmiller1990
Copy link
Member Author

lmiller1990 commented Dec 26, 2017

Thanks , I added those changes.

@eddyerburgh eddyerburgh merged commit 2819e51 into vuejs:dev Dec 26, 2017
@eddyerburgh
Copy link
Member

Thanks @lmiller1990 🙂

@lmiller1990
Copy link
Member Author

Thank you @eddyerburgh , the part about how setTimeout and task/microtasks was new to me. OSS is great. I feel like I've learned more doing OS in the last year than like 4 years at school at 2 in the workforce.

@Ray-56
Copy link

Ray-56 commented Jun 20, 2018

if
image had setTimeout 1500s, expected is error.
how do i handle it?

@lmiller1990
Copy link
Member Author

lmiller1990 commented Jun 20, 2018

@guokangf can you post your question on stackoverflow with vue-test-utils tag and the test you trying to write? I can help you there. This repo is just for issues with vue-test-utils.

Thanks.

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.

7 participants