-
Notifications
You must be signed in to change notification settings - Fork 67
Remove remaining wip pieces in tests #917
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
Remove remaining wip pieces in tests #917
Conversation
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 looks great, but im confused by the one line change that i pointed out...
@@ -87,30 +83,28 @@ def load_csv_impl(path, *args): | |||
mock_logger, | |||
csv_importer_impl=mock_csv_importer) | |||
|
|||
self.assertEqual(modified_row_count, 4) | |||
self.assertEqual(modified_row_count, 1) |
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.
i know this isnt exactly recent in your memory anymore, but why did this change to 1 instead of to 3??
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.
good question, let me look around. since tests pass, there must've been some change to this test in v4.
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.
thanks for pointing this out! i made a fix, so it's 3 now. here's an explanation:
- we have a database mock object, which overwrites
insert_or_update_bulk
- we didn't even count how many rows this function was passed, we just updated the mock
return_value
argument - so i rewrote this so
insert_or_update_bulk = MagicMock(wraps=iter_len)
whereiter_len
is just a function that counts the number of items in the iterable passed toinsert_or_update_bulk
. so now we correctly getmodified_row_count == 3
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.
👍
A few tests in #903 still had WIP references. Removed those.