-
Notifications
You must be signed in to change notification settings - Fork 264
Feature request: Clear events returned by "VueWrapper.emitted" method (similar to Jest mockClear) #1363
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
Comments
Could be interesting. @lmiller1990 WDYT? |
This comes up every so often and the answer is always the same; why is this better than just writing another test with a new Historical thread (I'm sure there are others): vuejs/vue-test-utils#1252 The maintainers opinion was not the most popular at the time (see the thread). What does Angular do for this @cexbrayat? Is there some kind of "reset" or is the standard just create another test/mount a new component? This is what React does, iirc. This could certainly be implemented using our plugins API? I feel like this feature is only useful for a subset of people, who could easily be served by a copy + paste plugin (which could also be distributed via npm). Always good to consider new features. I think we should have a hard stop on absolutely no API changes until 2.0 is out, though (two remaining failing tests: #1293), after which we can look into more enhancements. I like to keep the API surface pretty small; even as it stands, the code base is pretty complex and doesn't have a ton of active contributors - more code always leads to move maintenance overhead. What do you guys think? If there's a really compelling case for resetting emitted events, please point it out - happy to have a discussion about this feature. From what I can see it's just a convenience thing, and since it can be done quite easily in a plugin (I think?) that might be the best alternative. |
Thx for the hint. I'll examine it. |
My understanding is that if you const mocked = jest.mock('someFn') In your test, there is literally no way to "unmock" or "reset" it. Whatever changes is persistent for all tests. This isn't the case with a Vue component, since you can just remount it or write a new test. They are not globally registered like Jest mock. So that's why
What do your tests looks like? Wouldn't it just be function mountComponent () {
return mount(QuasarComponent, { /* 63 props */ }
}
it('case A', () => {
const wrapper = mountComponent()
wrapper.find('#a').trigger('click')
expect(wrapper.emitted()[aa']).toEqual([ /* something *] ])
})
it('case B', () => {
const wrapper = mountComponent()
wrapper.find('#b').trigger('click')
expect(wrapper.emitted()['b']).toEqual([ /* something *] ])
}) A tiny bit more code than you'd have with Sorry for so many questions - just wanting to see if there's really a use case here I'm missing. |
Edit: I tried to implement this as a plugin but it's not currently possible - Happy to leave this as a feature suggestion but unless there's a case where there's a test you cannot write with VTU, I don't think we should add this, at least certainly not before 2.0.0 - the current goal is to reach a stable release before adding new features. |
For anyone who has the same problem below is my temporary solution: interface WrapperExtension {
readonly clearEmitted: () => void;
}
type ExtendedWrapper<T extends VueWrapper> = T & WrapperExtension;
function extendWrapper<T extends VueWrapper>(wrapper: T): ExtendedWrapper<T> {
Reflect.set(wrapper, "clearEmitted", (): void => {
for (const events of Object.values(wrapper.emitted())) events.length = 0;
});
return wrapper as ExtendedWrapper<T>;
}
// Use it this way:
const wrapper = extendWrapper(mount(Component));
wrapper.clearEmitted(); |
Oh neat - maybe we can implement this as an option plugin. I forgot about |
Was there any reason you can't use my original suggestion of a factory function to make creating a new wrapper more concise? I'd like to either decide 1) we need this feature (not a fan) or 2) close if we don't need it. I've been burned by adding a feature that's only useful for a small number of use cases, which is why my default is "push back until we have a very compelling use case". |
Ok, here is the problem again: test("Sample test", () => {
const wrapper = vueTestUtils.mount(Table, {
global: testUtils.globalMountOptions(),
props:
columns: [
{
align: "left",
field(row) {
return cast.string(reflect.get(row, "name"));
},
label: "Sample label",
name: "column",
sortable: true
}
],
pagination: {
descending: false,
page: 5,
rowsPerPage: 0,
sortBy: "column"
},
rowKey: "id",
rows: [
{ id: "key1", name: "Sample row 1" },
{ id: "key2", name: "Sample row 2" },
{ id: "key3", name: "Sample row 3" },
...
]
});
wrapper.find(".first").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.find(".prev").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.find(".next").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 6,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 6,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 10,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.find(".page-7").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 6,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 10,
rowsPerPage: 0,
sortBy: "column"
}
],
[
{
descending: false,
page: 7,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}); My solution: test("Sample test", () => {
const wrapper = vueTestUtils.mount(Table, {
global: testUtils.globalMountOptions(),
props:
columns: [
{
align: "left",
field(row) {
return cast.string(reflect.get(row, "name"));
},
label: "Sample label",
name: "column",
sortable: true
}
],
pagination: {
descending: false,
page: 5,
rowsPerPage: 0,
sortBy: "column"
},
rowKey: "id",
rows: [
{ id: "key1", name: "Sample row 1" },
{ id: "key2", name: "Sample row 2" },
{ id: "key3", name: "Sample row 3" },
...
]
});
wrapper.find(".first").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.clearEmitted();
wrapper.find(".prev").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.clearEmitted();
wrapper.find(".next").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 6,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.clearEmitted();
wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 10,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
wrapper.clearEmitted();
wrapper.find(".page-7").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 7,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}); Your solution with factory: test("Sample test", () => {
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
function createWrapper() {
return vueTestUtils.mount(Table, {
global: testUtils.globalMountOptions(),
props:
columns: [
{
align: "left",
field(row) {
return cast.string(reflect.get(row, "name"));
},
label: "Sample label",
name: "column",
sortable: true
}
],
pagination: {
descending: false,
page: 5,
rowsPerPage: 0,
sortBy: "column"
},
rowKey: "id",
rows: [
{ id: "key1", name: "Sample row 1" },
{ id: "key2", name: "Sample row 2" },
{ id: "key3", name: "Sample row 3" },
...
]
});
}
{
const wrapper = createWrapper();
wrapper.find(".first").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 0,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}
{
const wrapper = createWrapper();
wrapper.find(".prev").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 4,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}
{
const wrapper = createWrapper();
wrapper.find(".next").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 6,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}
{
const wrapper = createWrapper();
wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 10,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}
{
const wrapper = createWrapper();
wrapper.find(".page-7").trigger("click");
expect(wrapper.emitted("update:pagination")).toStrictEqual([
[
{
descending: false,
page: 7,
rowsPerPage: 0,
sortBy: "column"
}
]
]);
}
}); Why I prefer "clearEmitted" method:
function clearEmitted(wrapper: VueWrapper): void {
for (const events of Object.values(wrapper.emitted())) events.length = 0;
} I still prefer to have official solution because the above function is a hack that may stop working in the future. |
Why not just wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")[0]).toStrictEqual(/*...*/)
wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")[1]).toStrictEqual(/*...*/)
wrapper.find(".last").trigger("click");
expect(wrapper.emitted("update:pagination")[2]).toStrictEqual(/*...*/) It's the same amount of lines of code. What do you think? Did you consider just indexing the Again - just pushing back to make sure we aren't adding a specific method for a niche use case. It looks like you can write the same test in as concise a method by just indexing the emitted events. I'm not pushing back because I don't like the idea of new features for a more ergonomic testing experience, I'm pushing back because it looks like you can accomplish what you want with the existing API. |
const callback = jest.fn();
callback("abc1");
expect(callback).nthCalledWith(0, "abc1");
callback("abc2");
expect(callback).nthCalledWith(1, "abc2");
callback("abc3");
expect(callback).nthCalledWith(2, "abc3"); So, according to your logic, they should remove "mockClear". However, they let me do this: const callback = jest.fn();
callback("abc1");
expect(callback).toHaveBeenCalledWith("abc1");
callback.clearMock();
callback("abc2");
expect(callback).toHaveBeenCalledWith("abc2");
callback.clearMock();
callback("abc3");
expect(callback).toHaveBeenCalledWith("abc3");
callback.clearMock(); The last sample has even one extra line per block (3 lines vs 2), but these blocks are completely independent and interchangable which is IMHO an advantage. Mabe it is possible to attach some kind of vote to this thread. |
I think you've made a reasonable case for the feature. I don't agree with it, but I don't think it's unreasonable, so happy to solicit a little more feedback from the wider community. Generally people will 👍 something they like, maybe we can wait a little and see if anyone else feels strongly about this feature. If so, we can look to implement it. A good first start to move this forward would be to actually create a PR adding this feature - it'll give more visibility, so people can vote on it, and we'll see if there's any unexpected complexity. What do you think? Would you be interested in this? Once we've seen the technical complexity, I can share this on Twitter (PR or issue) and get some more 👀 on it. |
This is the biggest gripe I have against resetting BUT what I am curious about is an improved API for
|
I'd like to see some way to inject |
I think it is already easy possible to use a spy function with emitted, when it's passed as props and const TestComponent = defineComponent({
emits: ['click'],
template: '<button @click="click">Click</button>',
setup(props, { emit }) {
return {
click: () => emit('click', 'some-value')
}
}
})
it('should check emit with spy function', async () => {
const onClick = vi.fn()
const wrapper = mount(TestComponent, {
props: {
onClick
}
})
await wrapper.find('button').trigger('click')
expect(onClick).toHaveBeenCalledTimes(1)
expect(onClick).toHaveBeenCalledWith('some-value')
onClick.mockClear()
expect(onClick).not.toHaveBeenCalled()
await wrapper.find('button').trigger('click')
expect(onClick).toHaveBeenCalledTimes(1)
expect(onClick).toHaveBeenCalledWith('some-value')
}) |
I recently used the same trick that @freakzlike commented above, and I think it does a good enough job instead of complicating the VTU APIs for this use case 👍 |
Is your feature request related to a problem? Please describe.
Consider the following code:
I would like to be able to write it this way:
Describe the solution you'd like
Add "clearEmitted" methods to "VueWrapper" interface.
The text was updated successfully, but these errors were encountered: