[DF-314] Adding method to drop columns from a table #30
Conversation
jufreire
added
documentation
|
|
||
| mocked_return_get_table = Mock() | ||
| mocked_return_get_table.sd.cols = [] | ||
| mocked_get_table.return_value = mocked_return_get_table |
There was a problem hiding this comment.
in this test the get table returns a table with no columns, then you try to delete the cols ["col1", "col2"]. This test could be improved because it isn't testing the cols list that is being removed.
I think this test could have a parameterize that tests the callings of mocked_alter_table with the cols list. To test your for (this test is missing).
About the for looping (from lines 100 to 104) testing: if you think that it applies you can move the for into a separate function and test it separately too.
|
|
||
| # assert | ||
| mocked_get_table.assert_called_once_with(dbname=db_name, tbl_name=table_name) | ||
| mocked_alter_table.assert_called_once_with( |
There was a problem hiding this comment.
I dont get how this test is working. The new_tbl arg from mocked_alter_table should not be equal mocked_return_get_table because the new_table should have only the column col3.
There was a problem hiding this comment.
As I could see running the test, pytest doesn't validate the Mock object properties.
This is the reason it's working (even with the mocked_return_get_table in new_tbl having the 3 columns) .
But, since you added the assert in line 125, I think it's ok =]
There was a problem hiding this comment.
I suggest we fix this, since it is wrong anyway :(
I will add a commit to adjust this test
|
Kudos, SonarCloud Quality Gate passed!
|
Why? 📖
We want to encapsulate logic to drop columns from a table in hive using thrift client classes.
What? 🔧
Type of change 🗄️
How everything was tested? 📏
Running locally and with unit tests.
Checklist 📝