Adds tests for rooms #181
Adds tests for rooms #181
Conversation
|
|
There was a problem hiding this comment.
Huge thanks 🙏 @hbcarlos
I have only cosmetic comments.
Regarding the CI, it will be good to fix the test on Windows - I don't know right away why they are failing. The only thing I figure out is that the failing step should not be named pypy and Windows but pypy or Windows 😝
| id = "test-id" | ||
| content = "test" | ||
| paths = {id: "test.txt"} | ||
| cm = FakeContentsManager({"content": content}) | ||
| loader = FileLoader( | ||
| id, | ||
| FakeFileIDManager(paths), | ||
| cm, | ||
| poll_interval=0.1, | ||
| ) |
There was a problem hiding this comment.
I'm wondering if this should be a fixture or a factory to avoid too many copy paste.
Fully optional - happy to merge as is.
There was a problem hiding this comment.
I looked at it again. Even though it seems like code duplication, it is not. Each test uses a different argument, and we need to create and parameterize multiple fixtures.
Let's merge the PR, and I'll work on creating a proper set of helpers and fixtures for testing.
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
|
Yes, I'll try to fix the Windows issue. |
|
Thanks a lot for making this one green @hbcarlos |
No description provided.