-
Notifications
You must be signed in to change notification settings - Fork 65
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
Remove enzyme #3243
Remove enzyme #3243
Conversation
zatteo
commented
Nov 7, 2024
BundleMonFiles updated (2)
Unchanged files (16)
Total files change +5.12KB +0.11% Groups updated (2)
Unchanged groups (5)
Final result: ✅ View report in BundleMon website ➡️ |
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 que tu as mis les différents exemples dans la carte ? Peut-être que lier la PR suffit remarque... ou lier les cartes entre elles
{ isEncryptedFolder: false } | ||
) | ||
}) | ||
it('should dispatch a createFolder action on submit', () => { |
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.
on devrait laisse le describe pour apporter du contexte / isoler le test non ?
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.
Avant il y avait deux describe
imbriqués, donc ce test est encore dans un describe('AddFolder', ...)
CURRENT_FOLDER_ID, | ||
{ isEncryptedFolder: false } | ||
expect.anything(), | ||
expect.anything() |
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 compte là et vu le titre du test on peut peut-être simplement faire toHaveBeenCalled()
Pourquoi est-ce qu'on supprime la partie vault d'ailleurs ?
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.
En fait le test précédent couvrait énormément de choses mais mal, tandis que maintenant on ne teste plus que simplement le composant AppFolder (l'objectif du test initial ?). C'est pour ça qu'il n'y a plus besoin de la partie vault
.
J'ai renommé le test qu'il corresponde mieux, mais je préfère garder le toHaveBeenCalledWith
pour pouvoir tester le 1er argument.
J'avais déjà mis chaque PR dans les cartes, et toutes les cartes sont liés par la milestone. Mais j'en ai profité pour aussi rajouter les liens de PR dans le paper 👍 |
31751e7
to
2785d3a
Compare