Conversation
…atabase-matrix into fractal-database-matrix-representations-test
justin-russell
left a comment
There was a problem hiding this comment.
Looks good for the most part. I see that there are a few places where we need to do a bit more assertions. The integration tests that are ensuring things like spaces are being created look great!
|
|
||
| # Broken with Update | ||
| """ | ||
| async def test_create_room_representation_success(): |
There was a problem hiding this comment.
This still broken? What was wrong with it?
| return_value=MagicMock(room_id="room_id"), | ||
| ), patch( | ||
| "builtins.print" | ||
| ) as mocked_print: |
There was a problem hiding this comment.
Let's avoid mocking print. There should be some way to ensure that the function you called did something. In this case, it should have called room_create
| # Broken on code update | ||
| # not important | ||
| """ | ||
| async def test_add_subspace_success_print(): |
|
|
||
| matrix_space = MatrixSpace() | ||
|
|
||
| # target.matrixcredentials_set.all.return_value = [] # Mocking empty matrix credentials |
There was a problem hiding this comment.
| # target.matrixcredentials_set.all.return_value = [] # Mocking empty matrix credentials |
|
|
||
| create_subspace = MatrixSubSpace.create_representation_logs(instance, target) | ||
| assert len(create_subspace) == 2 | ||
| print(create_subspace[0].method) |
There was a problem hiding this comment.
| print(create_subspace[0].method) |
| assert "room_id" in result | ||
|
|
||
|
|
||
| def test_create_representation_logs( |
There was a problem hiding this comment.
This test looks almost complete.
MatrixSubSpace.create_representation_logs(instance, target) returns two items. We should verify what those two items are. They should be two representation logs, one to create a space for the instance, then another that adds the created space to the parent space.
| assert create_subspace[0].instance == instance | ||
|
|
||
|
|
||
| async def test_create_subspace_reprsentation( |
There was a problem hiding this comment.
Clear out the print statements in this test
| assert target.metadata["room_id"] in room_ids | ||
|
|
||
|
|
||
| def test_create_subroom_reprsentation( |
There was a problem hiding this comment.
As I mentioned in test_create_representation_logs, you should verify what the two items in the result are.
|
|
||
| result = subspace.create_representation_logs(instance=target, target=primary_target) | ||
| # add an assert | ||
| assert result[0].instance == target |
There was a problem hiding this comment.
Let's make sure to assert what we expect the .method to be, etc.
Unit tests for representations.py and a couple integration tests. Names could probably be changed for tests for clarity. also some tests broke after an pull from main.