-
Notifications
You must be signed in to change notification settings - Fork 61
Picker tests: set data-app attribute in template instead of body in beforeAll hook #960
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
Conversation
That vue does not throw errors when comparing snapshots.
E*Picker.spec.js: add attachTo: document.body
await wrapper.find('button').trigger('click') | ||
expect(document.body).toMatchSnapshot('pickeropen') | ||
expect(wrapper).toMatchSnapshot('pickeropen') | ||
wrapper.destroy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ist wrapper.destroy()
in jedem Test nötig der attachTo: document.body
verwendet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say so. The documentation says (https://vue-test-utils.vuejs.org/api/options.html#attachto):
When attaching to the DOM, you should call wrapper.destroy() at the end of your test to remove the rendered elements from the document and destroy the component instance.
However, it seems that in other tests using attachTo: document.body
there are currently no side-effects of not calling destroy
. But I think we should do it anyway.
wrapper.destroy() | ||
}) | ||
|
||
test('updates v-model when the value changes', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weshalb kommt dieser Test ohne data-app
aus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The picker is never opened in this test (and others), thus we don't need a <div data-app>
where Vuetify can attach the picker to.
class="v-slider__thumb grey lighten-2" | ||
/> | ||
</div> | ||
<div data-app> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seltsam dass sich die Formatierung so stark unterscheidet, nur weil jetzt <body>
nicht mehr da ist... Aber okay solange es nicht ständig hin- und her ändert.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect that most of the formatting changes arise from changing expect(document.body).toMatchSnapshot('pickeropen')
to expect(wrapper).toMatchSnapshot('pickeropen')
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because not the jest-serializer-vue-tjw is used if you pass document.body to match the snapshot.
> | ||
<input | ||
disabled="disabled" | ||
id="input-26" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you see the id, which would be removed if jest-serializer-vue-tjw was used.
No description provided.