Updated example TodoApp to script setup #2727
Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
examples/TodoApp.spec.js
Outdated
|
|
||
| expect(wrapper.findAll('[data-test="todo"]')).toHaveLength(2) | ||
| }) | ||
| it('should not creates duplicate todo', async () => { |
There was a problem hiding this comment.
creates -> create
duplicate -> duplicated
examples/TodoApp.vue
Outdated
| id: number; | ||
| text: string; | ||
| completed: boolean; | ||
| }; |
There was a problem hiding this comment.
I would use an interface named Todo here:
interface Todo {
id: number;
examples/TodoApp.vue
Outdated
| v-for="todo in todos" | ||
| :key="todo.id" | ||
| data-test="todo" | ||
| :class="getClass(todo.completed)" |
There was a problem hiding this comment.
I would use: `:class="{ complete: todo.completed }" instead
There was a problem hiding this comment.
I will check it out. I just refactored what was there @cexbrayat
Thank you
examples/TodoApp.vue
Outdated
| const createTodo = () =>{ | ||
| const listLength = todos.value.length; | ||
| const lastItem = todos.value[listLength - 1];// get last todo item | ||
| const todoExist = todos.value.find(res => res.text.toLowerCase() === newTodo.value.toLowerCase());//check is todo already exists |
There was a problem hiding this comment.
I think we should keep the example as simple as it was and remove this
There was a problem hiding this comment.
I agree the other is simple, but If we kept it the same, it wouldn’t act like a real TODO list — we’d end up with duplicate texts and IDs. So I added a small tweak to make the tests more realistic.
@cexbrayat
There was a problem hiding this comment.
I don't think it's important to be realistic, a simple version is enough 👍
| @@ -1,32 +1,49 @@ | |||
| /** | |||
| * This is the example app used in the documentation. | |||
| * If you want to run it, you will need to build the final bundle with | |||
| * yarn build. Then you can run this with `yarn test examples` | |||
| * pnpm build. Then you can run this with `pnpm test examples` | |||
There was a problem hiding this comment.
I think the test file should be in TypeScript (and all references in docs should be updated accordingly)
There was a problem hiding this comment.
and all references in docs should be updated accordingly
Can you explain what you mean? @cexbrayat
There was a problem hiding this comment.
In case this file is referenced somewhere in the docs, but maybe it isn't 👍
| expect(wrapper.find('[data-test="todo"]').text()).toBe('Learn Vue.js 3') | ||
| }) | ||
| expect(wrapper.find('[data-test="todo"]').text()).toBe('Learn Vue.js 3') | ||
| expect(wrapper.findAll('[data-test="todo"]')).toHaveLength(1) |
There was a problem hiding this comment.
nit: in real life, I would check the number of todos before checking the text, so I would switch the order of these 2 lines
|
|
||
| await wrapper.find('[data-test="todo-checkbox"]').setChecked() | ||
| await wrapper.find('[data-test="todo-checkbox"]').setChecked() |
There was a problem hiding this comment.
I would also check that the class is not on the element before this line
examples/TodoApp.vue
Outdated
| const todoExist = todos.value.find(res => res.text.toLowerCase() === newTodo.value.toLowerCase());//check is todo already exists | ||
| if(newTodo.value && !todoExist) { | ||
| const id = lastItem.id + 1;//increment the last item id by 1 |
There was a problem hiding this comment.
there should a space before the comment, but it's weird that prettier did not catch it
2 participants
I noticed a few things which I am not sure if it was delibrate
describeanditjust like all the tests on the projectI noticed I couldn't run the test until I included the path to where the todo app test is, like so: