Skip to content

Conversation

@Kawacrepe
Copy link
Member

@Kawacrepe Kawacrepe commented May 21, 2022

Hey!

Here is a PR, adding test around unsafeCallee.

It aims to add some tests around probes, according to this issue #24, but as you may understand, I do not want to add all tests case inside 1 PR.

Furthermore, it would allow us to onboard people on js-x-ray, since they will get an example, about how we want to test the probes.

@Kawacrepe Kawacrepe changed the title Add ut around probes Add tests around probes May 21, 2022
@Kawacrepe
Copy link
Member Author

@fraxken Hey, before adding others tests, I wanna know if I'm on the right path (since I just get into this project :) )
Is that the kind of test we want ?

@Kawacrepe
Copy link
Member Author

Kawacrepe commented May 25, 2022

@fraxken Hey, est-ce que tu valides le flow de test ?

Je pense qu'à terme on aura des petits utils pour vérifier les comportements spécifiques de certaines probes (je pense à isMemberExpression qui ajoute des dependencies, isBinary qui tape sur les .counter etc...

Mais ça montre l'idée générale et permettra de faire des petites PR pour ajouter des cas de tests, sans avoir à trop se prendre la tête, non?

EDIT: je pense qu'il serait même intéressant d'ajouter un README.md dans le dossier des tests pour expliquer le flow, les différents tests etc...
Vu qu'on dit souvent que les tests sont une certaine forme de doc du projet

@fraxken
Copy link
Member

fraxken commented May 26, 2022

Yes, seems ok to me!

@Kawacrepe Kawacrepe self-assigned this May 27, 2022
@Kawacrepe Kawacrepe marked this pull request as ready for review May 27, 2022 12:41
@Kawacrepe Kawacrepe requested a review from a team May 27, 2022 12:42
@fraxken fraxken merged commit 753c4ec into master May 30, 2022
@fraxken
Copy link
Member

fraxken commented May 30, 2022

@all-contributors please add @Kawacrepe for code, test

@allcontributors
Copy link
Contributor

@fraxken

I've put up a pull request to add @Kawacrepe! 🎉

@fraxken fraxken deleted the add-ut-around-probes branch June 2, 2022 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants