DeepLcom / sql-mock

A Python library to test your SQL models using mocked input data
MIT License
34 stars 5 forks source link

Allowing more flexibility for developer assertions #45

Open fgarzadeleon opened 9 months ago

fgarzadeleon commented 9 months ago

When using the library the following I thought of the following change.

Problem: Currently the assertion to check if the data is correct is within the table_mocks.py when for a Mock you run assert_equal. When my assertion is not equal I don't receive more information about why they don't match.

Proposal: It would be better to do the following:

  1. Separate the assertion from the generation of the query and data.

Currently assert_equal also has responsibility over running _generate_query and _get_results. I see benefits from separating these. We could have a method called generate_results so that we can access the query and result directly from the Mock object.

  1. Separate the cleaning logic in _assert_result and move it generate_results this way this usage could look like this:
# assuming we've successfully ran TableMock.from_mocks() we currently do

TableMock.assert_equal(expected_dict)

# I propose the following which allows de the developer to use other assertion tools
res_dict = TableMock.generate_result()

query = TableMock._sql_mock_data.last_query # I can inspect the query even before it fails if I want to

unittest.TestCase.assertDictEqual(res_dict, expected_dict) # I do my own assertion with whatever library I chose

In this case the more verbose unittest assertion solves my problem.

Conclusion In a way I think BaseTableMock does not need an assert method within the object, you can expose the data and let the developers write their own assertion statement.

Somtom commented 9 months ago

Good points. Thanks for flagging that.

I was wondering whether you set things up using the recommended Pytest setup including pytest-icdiff? This should already show a rich comparison of what is not equal in a git diff way.

Currently, the intended behavior is that the used query should be printed out when the assertion fails. Did this not happen to you?

I like the idea of being able to manually call generate_results if needed. However, your proposed solution adds some additional verbosity (at least 1 additional line needed to do the assertion). So I would love to keep the possibility to call assert_equal but maybe provide the feature to be able to only get the results and do something custom with them.

fgarzadeleon commented 9 months ago

was wondering whether you set things up using the recommended Pytest setup including pytest-icdiff? This should already show a rich comparison of what is not equal in a git diff way.

I did not thanks, this would fix my main issue actually.

Currently, the intended behavior is that the used query should be printed out when the assertion fails. Did this not happen to you?

Yeah this works thanks! I guess the issue is that I have to copy the query and run it separately to get the answer. But as you said installing pytest-icdiff gives me better assertion error messages.

I like the idea of being able to manually call generate_results if needed. However, your proposed solution adds some additional verbosity (at least 1 additional line needed to do the assertion). So I would love to keep the possibility to call assert_equal but maybe provide the feature to be able to only get the results and do something custom with them.

The pattern I see in other libaries is that testing methods are not part of the main object you are working with, for example in pandas testing is in its own module: https://pandas.pydata.org/docs/reference/testing.html