Skip to content
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

a jest testing poc #46

Closed
wants to merge 2 commits into from
Closed

a jest testing poc #46

wants to merge 2 commits into from

Conversation

ivmos
Copy link
Contributor

@ivmos ivmos commented Oct 24, 2018

Hi friends

I've tried to do some basic tests for mediasoup-client, it can be challenging and fun because of the way it's architected.

Probably this PR is not good enough for merging, I might even break something as I needed to mess with deps, but I wanted to show what I tried and this serves as PR++ for Hacktoberfest even if not accepted. Perhaps it can help as reference if adding unit tests is scoped in the future.

Thank you and happy Hacktoberfest :)

captura de pantalla 2018-10-24 a las 22 21 46

@ibc
Copy link
Member

ibc commented Nov 4, 2018

Hi, this is definitely a must. We need test units in mediasoup-client.

Said that, it's important to test the mediasoup-client surface API, but it's even more important to test that the underlying stuff (per-browser specific handlers) do work as expected.

Let's discuss about this next week with tons of beers.

@ibc
Copy link
Member

ibc commented Nov 28, 2018

Hi, we are planning and defining mediasoup v3. In both, client and server sides, v3 will expose a much more granular API that will make it possible to add more test units as those you propose here.

Will come back to this once v3 is done.

@ibc
Copy link
Member

ibc commented Dec 1, 2018

Hey, I've added jest in mediasoup-client v3 branch: ee43dd3

So thanks very useful :)

@ibc ibc closed this Dec 1, 2018
@ivmos
Copy link
Contributor Author

ivmos commented Dec 2, 2018

Great thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants