-
Notifications
You must be signed in to change notification settings - Fork 0
Upload file test #6
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
base: main
Are you sure you want to change the base?
Conversation
…atabase-matrix into fractal-database-matrix-representations-test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this test?
|
||
matrix_space = MatrixSpace() | ||
|
||
# target.matrixcredentials_set.all.return_value = [] # Mocking empty matrix credentials |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print(create_subspace[0].method) |
assert "room_id" in result | ||
|
||
|
||
def test_create_representation_logs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.