Webrtc refactor

[gjmooney]

Split up webrtc.js into separate files based on the type of stream.

[martinRenou]

Thanks!

[maartenbreddels]

Nice, I'm generally ok with large files, and prefer them actually, but maybe this file was a bit too much.

My only worry is with all of these changes is that we break ipywebrtc without noticing. I'm not a fan (anymore) of galata based testing, and we've recently switch over to using solara for testing. This is a pure pytest based solution, so it's much simpler for people to adopt.

The docs are on here: https://solara.dev/docs/howto/testing

An example that only tests the DOM on all major widget platforms (notebook/lab/voila/solara) can be found:

• https://github.com/widgetti/ipyaggrid/blob/master/tests/ui/jupyter_test.py
• https://github.com/widgetti/ipyreact/blob/master/tests/ui/module_test.py

A test that only runs on solara (much faster) can be found:

• https://github.com/widgetti/ipyreact/blob/master/tests/ui/event_test.py

I prefer tests to only test the dom structure and possibly events, I don't think we need to store screenshots for now, a basic smoke test should be enough. In the future we could possibly test the screenshot feature etc, but that is more work.

We set up tests usually like this:

• https://github.com/widgetti/ipyreact/blob/af986ed203f53d5ce6ad8aacbed46f93877a4b89/.github/workflows/build.yml#L81

So that the install and test are separate jobs.

If you have the bandwidth to add UI testing, let me know, but I thought let me at least share our vision on testing with you and @martinRenou . Possibly this could be interesting for other ipywidgets projects as well.

[martinRenou]

This is a pure pytest based solution, so it's much simpler for people to adopt.

Sounds exciting! I'm in for testing widgets using this.

The only downside of it is that it does not test widgets in the JupyterLab context, which may break while the solara case still works. Though let's not maintain two test approaches, enough tooling.

You're right that we should add ui-tests before making any big change to this. @gjmooney would you be willing to add solara tests?

[maartenbreddels]

If you use the ipywidgets_runner fixture, we test it on JupyterLab+Notebook (6)+Voila+Solara, as done in:
https://github.com/widgetti/ipyaggrid/blob/master/tests/ui/jupyter_test.py

[gjmooney]

Yea, I can look into adding/updating the tests.

[maartenbreddels]

Nice, good to see a test in here!

[maartenbreddels]

Suggested change

	vid = page_session.locator(".video-stream").wait_for()
	playwright.sync_api.expect(vid).to_be_visible()
	page_session.locator(".video-stream").wait_for()

wait_for returns None, you could have done:

    vid = page_session.locator(".video-stream")
    vid.wait_for()
    playwright.sync_api.expect(vid).to_be_visible()

But that basically tests the same thing (see docstring of wait_for)

[maartenbreddels]

Please take a look at my latest suggestion regarding the failure

[maartenbreddels]

Does it pass locally on your computer?

[gjmooney]

Does it pass locally on your computer?

Nope. I'm thinking the issue is with the file location, if the kernel_code() is running in a separate kernel I'm assuming it can't find that file? I ran the tests in with --headed and all the tests show Error creating view for mediastream: cannot play video stream.

I can get tests involving CameraStream to pass, but only in headed mode since it's requesting the webcam permission. Does Solara have a way to access Playwrights browser context to call grant_permissions?

[maartenbreddels]

Nope. I'm thinking the issue is with the file location, if the kernel_code() is running in a separate kernel I'm assuming it can't find that file?

I recently had this issue, I'll get back to you on this with a way to pass the location.

Does Solara have a way to access Playwrights browser context

You can use all the playwright fixtures, such as browser etc, not that we use page_session instead of page to get a shared single page.

[maartenbreddels]

Note that you can pass argument to the function, see https://github.com/widgetti/ipyreact/blob/b500a53bcf4883627d724281d4f9b4e2b59e5798/tests/ui/module_test.py#L136

But note that in one of the tests (using solara server) the arguments do not get passed, you can do this workaround for now:
https://github.com/widgetti/ipyreact/blob/b500a53bcf4883627d724281d4f9b4e2b59e5798/tests/ui/module_test.py#L47