Conversation
Pull Request Test Coverage Report for Build 196497422
💛 - Coveralls |
ccd8184 to
040166d
Compare
| ): | ||
| if is_chromium and is_win: | ||
| pytest.skip("see https://github.com/microsoft/playwright/issues/717") | ||
| return |
There was a problem hiding this comment.
I need to add the return in the two cookie tests from yesterday. Good catch!
There was a problem hiding this comment.
Actually I'm not sure if we need it! It was flaky so I added it. But seems like its not needed: https://www.programcreek.com/python/example/25423/pytest.skip
| def pytest_collection_modifyitems(items): | ||
| for item in items: | ||
| item.add_marker(pytest.mark.asyncio) | ||
|
|
There was a problem hiding this comment.
Can you explain to me why this is needed? (I'm new to async in Python.)
There was a problem hiding this comment.
Before it was in the global conftest.py. A conftest.py will be executed through the directory three and by that they can overwrite each other (which is nearest to the actual test).
This specific piece added asyncio support to the tests which was before available in all the tests. But since we split async and sync I limited the scope of it.
There was a problem hiding this comment.
Thanks! So async tests need to be explicitly (or programmatically) marked as async for pytest to run them?
There was a problem hiding this comment.
exactly with: https://github.com/pytest-dev/pytest-asyncio
You can do it manually with the decorator per test or in our case just for all the tests in that directory.
| noWaitAfter: bool = None, | ||
| ) -> None: | ||
| await self._channel.send("text", locals_to_params(locals())) | ||
| await self._channel.send("type", locals_to_params(locals())) |
There was a problem hiding this comment.
I'm so happy porting tests reveal this.
No description provided.