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

2.0.0-rc.6 Poor error handling with promises & empty wrapper #638

Closed
Giwayume opened this issue Jun 1, 2021 · 5 comments · Fixed by #641
Closed

2.0.0-rc.6 Poor error handling with promises & empty wrapper #638

Giwayume opened this issue Jun 1, 2021 · 5 comments · Fixed by #641

Comments

@Giwayume
Copy link

Giwayume commented Jun 1, 2021

Subject of the issue

If I resolve an empty DOMWrapper as the return value of a promise, which is then awaited by an async function, get this error:

Cannot call then on an empty DOMWrapper.

Steps to reproduce

import { defineComponent, nextTick } from 'vue';
import { mount } from '@vue/test-utils';

async function findAfterNextTick(wrapper, selector) {
    await nextTick();
    return wrapper.find(selector);
}

it('some test', async () => {
    const wrapper = mount(
        defineComponent({
            template: `<div>My component</div>`
        })
    );
    const foundElement = await findAfterNextTick(wrapper, '.something-that-does-not-exist');
    expect(foundElement.exists()).toBeFalsy();
});

Expected behaviour

The test above should pass.

Actual behaviour

Getting this error.

Cannot call then on an empty DOMWrapper.

Possible Solution

This is way too generic.
https://github.com/vuejs/vue-test-utils-next/blob/ecf7905fd36a707174421f5c58b57e000faaf3e0/src/errorWrapper.ts#L11

I'm assuming nodejs is testing if the returned object is thenable, which causes this error. Fix so it's only checking for wrapper properties, just like V1 of @vue/test-utils. Denying all properties is going to lead to weird bugs like this. It may be nice for brevity in your codebase, but it wasn't a smart change.

@lmiller1990
Copy link
Member

How about this? #641. I added your test case.

@Giwayume
Copy link
Author

Giwayume commented Jun 2, 2021

That works too, I just worry there's always something you're going to be missing by piecemealing this way.

@lmiller1990
Copy link
Member

I'm not sure what "piecemealing" means, but this seems okay. If we run into more problems, we can consider something similar to VTU v1.

@Giwayume
Copy link
Author

Giwayume commented Jun 3, 2021

Piecemeal is to hand-pick, as in add excluded properties as problems are found. I prefer to write code that is safe under unknown conditions, which is the way v1 worked.

@lmiller1990
Copy link
Member

Thanks for clarifying that. I see your perspective too; if anything else comes up around this, let's revert to the more boilerplate heavy but reliable style of writing out each method.

I will release this patch soon.

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 a pull request may close this issue.

2 participants