#7058 LeftMenu vitest #1153

Merged
jsegarra merged 26 commits from 7058_leftMenu_vitest into dev 2025-02-06 21:45:16 +00:00
Member

Hay 3 skip todavia

Hay 3 skip todavia
jsegarra added 5 commits 2024-12-31 13:03:00 +00:00
jsegarra added 1 commit 2024-12-31 13:13:45 +00:00
gitea/salix-front/pipeline/pr-dev There was a failure building this commit Details
4618ba87fa
test: refs #7058 improve getRoutes
jsegarra added 7 commits 2025-01-03 15:28:41 +00:00
jsegarra added 1 commit 2025-01-14 04:51:22 +00:00
gitea/salix-front/pipeline/pr-dev This commit looks good Details
9d90d041ee
Merge branch 'dev' into 7058_leftMenu_vitest
jsegarra added 2 commits 2025-01-20 22:08:29 +00:00
jsegarra requested review from jorgep 2025-01-20 22:08:52 +00:00
jsegarra changed title from WIP: #7058 LeftMenu vitest to #7058 LeftMenu vitest 2025-01-20 22:08:55 +00:00
jorgep reviewed 2025-01-21 11:19:08 +00:00
@ -35,1 +35,4 @@
const filteredItems = computed(() => {
return filterItems();
});
function filterItems() {
Member

Solo se usa 1 vez

Solo se usa 1 vez
Author
Member

Lo sé, era la manera mas simple de probar la lógica de esa función.
Esto no debería haber subido así porque no haría falta poner las llaves y el return

Lo sé, era la manera mas simple de probar la lógica de esa función. Esto no debería haber subido así porque no haría falta poner las llaves y el return
Member

Si no hay manera de poder testearlo vale, pero si se puede mejor poner dentro de computed, si se vuelve muy complicado quizá sería mejor testearlo con test unitario con cypress

Si no hay manera de poder testearlo vale, pero si se puede mejor poner dentro de computed, si se vuelve muy complicado quizá sería mejor testearlo con test unitario con cypress
Member

Yo haría el test en un unitario con cypress. Queda feo crear una fn que solo vas a usar 1 vez y es simple.

Yo haría el test en un unitario con cypress. Queda feo crear una fn que solo vas a usar 1 vez y es simple.
Author
Member

He vuelto a probar y me ha funcionado dejandolo dentro del computed.
No se que pasó

He vuelto a probar y me ha funcionado dejandolo dentro del computed. No se que pasó
jsegarra marked this conversation as resolved
jorgep reviewed 2025-01-21 11:28:49 +00:00
@ -73,0 +166,4 @@
expect(getMethodB).not.toHaveBeenCalled();
});
it('should call getMethodA when source is main', () => {
Member

Poner otro nombre, está repetido.

Poner otro nombre, está repetido.
Author
Member

oh vaya

oh vaya
Member

should not call getMethodA when method is undefined por ej.

should not call getMethodA when method is undefined por ej.
jsegarra marked this conversation as resolved
jorgep reviewed 2025-01-21 11:35:29 +00:00
@ -93,0 +348,4 @@
vi.clearAllMocks();
});
it('should add menu items to parent if matches are found', () => {
Member

Aquí habría que comprobar que el módulo ha sido añadido no?

Aquí habría que comprobar que el módulo ha sido añadido no?
Author
Member

Eso correspondería al test de useNavigationStore, no? No podríamos ni deberíamos testear funcionalidad de otros archivos

Eso correspondería al test de useNavigationStore, no? No podríamos ni deberíamos testear funcionalidad de otros archivos
Member

En el título del archivo pone should add menu items. Como sabes que ha funcionado? solo sabes que se ha llamado a la función. Los 3 tests son exactamente iguales, con diferente título. Veo tu punto, si no crees que sea el lugar de testearlo, cambia el título del test por uno que compruebe realmente lo que hace esa función, llamar a otra función.

En el título del archivo pone should add menu items. Como sabes que ha funcionado? solo sabes que se ha llamado a la función. Los 3 tests son exactamente iguales, con diferente título. Veo tu punto, si no crees que sea el lugar de testearlo, cambia el título del test por uno que compruebe realmente lo que hace esa función, llamar a otra función.
Author
Member

Mmm...WTF.
Lo reviso, pero si 3 iguales

Mmm...WTF. Lo reviso, pero si 3 iguales
Member

Hacer solo 1 test que compruebe que la función ha sido llamada. Estos tests hay hacerlos en navigationStore según lo que hablamos en persona.

Hacer solo 1 test que compruebe que la función ha sido llamada. Estos tests hay hacerlos en navigationStore según lo que hablamos en persona.
jorgep marked this conversation as resolved
jorgep reviewed 2025-01-21 11:37:59 +00:00
@ -93,0 +323,4 @@
});
});
describe('addChildren', () => {
Member

En estos tests, solo veo que compruebes que la función haya sido llamada, no se debería comprobar si navigation ha cambiado?

En estos tests, solo veo que compruebes que la función haya sido llamada, no se debería comprobar si navigation ha cambiado?
Member

El test no comprueba que se ha añadido el item, solo que la función addMenuItem ha sido llamada. Quitar test y comprobar si se han añadido en los tests de useNavigationStore.

El test no comprueba que se ha añadido el item, solo que la función addMenuItem ha sido llamada. Quitar test y comprobar si se han añadido en los tests de useNavigationStore.
jorgep marked this conversation as resolved
jsegarra added 2 commits 2025-01-21 22:38:02 +00:00
jgallego removed review request for jorgep 2025-01-24 13:51:05 +00:00
jgallego requested review from jorgep 2025-01-24 13:51:10 +00:00
jorgep reviewed 2025-01-29 16:02:24 +00:00
@ -73,0 +194,4 @@
});
it('should filter items based on search input', async () => {
vm.search = 'Rou';
Member

Estaría bien crear un test que devuelva algún resultado, mockea otra para comprobar que solo te va a devolver esa.

Estaría bien crear un test que devuelva algún resultado, mockea otra para comprobar que solo te va a devolver esa.
Author
Member

He usado cust para que compruebe customer

He usado cust para que compruebe customer
jorgep marked this conversation as resolved
jorgep requested changes 2025-01-29 16:06:17 +00:00
Dismissed
jorgep left a comment
Member

.

.
jsegarra added 2 commits 2025-02-05 07:29:13 +00:00
gitea/salix-front/pipeline/pr-dev There was a failure building this commit Details
5e2158daf4
test: refs #7058 chnges requested
jsegarra added 2 commits 2025-02-05 12:55:51 +00:00
jsegarra added 1 commit 2025-02-05 12:56:02 +00:00
gitea/salix-front/pipeline/pr-dev There was a failure building this commit Details
b5b52dcda0
Merge branch 'dev' into 7058_leftMenu_vitest
jsegarra reviewed 2025-02-05 12:56:47 +00:00
@ -0,0 +55,4 @@
describe('useNavigationStore', () => {
beforeEach(() => {
setActivePinia(createPinia());
Author
Member

Si lo dejas aquí OK
Si lo mueves a beforeAll, tienes dependencia de los casos de prueba y fallan

Si lo dejas aquí OK Si lo mueves a beforeAll, tienes dependencia de los casos de prueba y fallan
jorgep marked this conversation as resolved
jsegarra requested review from jorgep 2025-02-05 12:56:52 +00:00
jorgep reviewed 2025-02-05 14:31:21 +00:00
@ -93,0 +328,4 @@
expect(vm.normalize(input)).toBe(expected);
});
});
// describe('addMenuItem', () => {
Member

Quitar

Quitar
Author
Member

OMG

OMG
jsegarra marked this conversation as resolved
jorgep reviewed 2025-02-05 14:35:17 +00:00
@ -0,0 +104,4 @@
expect(store.pinnedModules).toEqual(['order']);
});
it('should add menu item correctly', () => {
Member

yo comprobaría tambien que se ha añadido en el parent

yo comprobaría tambien que se ha añadido en el **parent**
Author
Member

Vaya, no lo veía importante, pero tras hacer unas pruebas, si que lo es

Vaya, no lo veía importante, pero tras hacer unas pruebas, si que lo es
jsegarra marked this conversation as resolved
jorgep requested changes 2025-02-05 14:37:46 +00:00
Dismissed
jorgep left a comment
Member

.

.
jsegarra added 2 commits 2025-02-05 22:03:48 +00:00
jsegarra requested review from jorgep 2025-02-05 22:04:32 +00:00
jorgep approved these changes 2025-02-06 13:18:59 +00:00
jsegarra added 1 commit 2025-02-06 21:44:30 +00:00
gitea/salix-front/pipeline/pr-dev This commit looks good Details
48e40cdc46
Merge branch 'dev' into 7058_leftMenu_vitest
jsegarra scheduled this pull request to auto merge when all checks succeed 2025-02-06 21:44:37 +00:00
jsegarra merged commit 4994a94369 into dev 2025-02-06 21:45:16 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: verdnatura/salix-front#1153
No description provided.