-
Notifications
You must be signed in to change notification settings - Fork 3
Init jest && config / add test component LcButton #20
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
5977cba to
3a97a4d
Compare
sbourdon13
left a comment
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.
| it('Should be a loader button', () => { | ||
| wrapper = mount(LcButton, { props: { loader: true } }) | ||
|
|
||
| expect(wrapper.html()).toMatchSnapshot() |
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.
ce serait peut-être pas mal de vérifier que la div du loader existe ? Hier en faisant de la review j'ai vu un snapshot sur leonardo "should display list of result" qui avant la pr ne contenait aucun résultat 😬 donc je pense que c'est pas mal d'assurer ses arrières quand on utilise le snapshot, comme pour les classes juste en-dessous
| }) | ||
|
|
||
| it('Must have a primary color as default', () => { | ||
| expect(wrapper.html()).toMatchSnapshot() |
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.
pourquoi ne pas vérifier la classe primary plutôt ?
| expect(wrapper.vm).toBeTruthy() | ||
| }) | ||
|
|
||
| it('Should be a loader button', () => { |
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.
est-ce qu'on mettrait pas des minuscules aux tests ?
| expect(wrapper.classes('lc-btn--grey')).toBeTruthy() | ||
| }) | ||
|
|
||
| it('Should have a black class color', 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.
est-ce qu'on pourrait pas faire une boucle pour les 5 derniers test ? ils ont la même structure, y a que le nom de la couleur qui change
| }) | ||
|
|
||
| it('Must have a primary color as default', () => { | ||
| expect(wrapper.html()).toMatchSnapshot() |
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.
idem autant tester la classe
| expect(wrapper.classes('lc-link--black')).toBeTruthy() | ||
| }) | ||
|
|
||
| it('Should have a white class color', 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.
idem est-ce qu'on peut faire une boucle
|
|
||
| it('Should be a variant outline', () => { | ||
| wrapper = mount(LcButton, { props: { variant: 'outline' } }) | ||
| // await wrapper.setProps({ variant: 'outline' }) |
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.
commentaire
| expect(wrapper.classes('lc-btn--lg')).toBeTruthy() | ||
| }) | ||
|
|
||
| it('Should have a xl class size', 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.
boucle pour ces tests ?
| }) | ||
|
|
||
| describe('Link', () => { | ||
| it('Must be a none button', () => { |
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.
je comprends pas ce que ça veut dire.
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.
je pas trop compris non plus, j'ai changé le wording du test
| @@ -1,5 +1,5 @@ | |||
| import LcTooltip from '../components/LcTooltip' | |||
| import LcButton from '../components/LcButton' | |||
| import LcButton from '../components/LcButton/LcButton' | |||
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.
est-ce qu'on pourrait mettre index.vue pour alléger l'écriture, comme on a fait pour donatello Search/index.vue, ou est-ce que pour storybook ça doit être cette architecture de fichiers LcButton/LcButton ?
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.
Je t'avoue la j'ai fais au plus rapide pour centralisé tout dans le même dossier les "tests" et le "composant".
Je regarderait dans un autre ticket je pense car il y aura peut-être de la configuration lié à storybook ;)
J'ai ajouté la couleur rouge elle n'avait jamais été prévu pour le bouton mais seulement pour l'icone j'ai l'impression |

Init Jest
alphaourcmais c'est les seuls qui fonctionnent), j'ai repris les versions installées sur leonardo (je crois que Joffrey avait géléré aussi à installer jest) car les dernières versions de certain packages genères des erreurs de compilation (vue-template-compiler)Création d'un premier test de composant (LcButton) mêmes tests que sur donatello, j'ai un peu refacto et corrigé des erreurs car certain test failed, surtout lié à la partie
linkil manquait des condition dans le code.Réorganisation du composant en créant un dossier LcButton qui contient le composant ainsi que les tests.
Configuration GithubAction pour les unit test
Mise en cache de yarn (node_modules) sur github actions https://github.com/actions/cache/blob/main/examples.md#node---yarn