search

VForWaTer / metacatalog

Modular metadata management platform for environmental data.
https://vforwater.github.io/metacatalog
GNU General Public License v3.0
3 stars 1 forks source link

Persons are added in test_groups_projects.py & test_api_add_find.py #116

Closed AlexDo1 closed 3 years ago

AlexDo1 commented 3 years ago

It caused some confusion, that a person is added to the persons table in https://github.com/VForWaTer/metacatalog/blob/007cbfe01831b8ce44131965645ecf4093df03d4/metacatalog/test/test_groups_projects.py#L12-L15 before the persons for the find tests are created here: https://github.com/VForWaTer/metacatalog/blob/007cbfe01831b8ce44131965645ecf4093df03d4/metacatalog/test/test_api_add_find.py#L9-L13 https://github.com/VForWaTer/metacatalog/blob/007cbfe01831b8ce44131965645ecf4093df03d4/metacatalog/test/test_api_add_find.py#L21-L29 I think it would be better if the person table would be "clean" and empty, to make it easy to search i.e. for an id. In my opinion, the test would also be more comprehensible this way.

AlexDo1 commented 3 years ago

If it is not necessary to test if a person can be found via ID, this issue is maybe obsolete.

mmaelicke commented 3 years ago

Thanks for checking all these tests. These tests were defined at very different times and are not interfering with each other. I think the order at which they execute is also not deterministic. From my point of view, it makes sense to test as many ways, how one would interact with the database in unit tests. The test_api_add_find suite is pretty much testing the add and find api, while the test_groups_projects is only asserting if the the Grouping of Metadata is working correctly. The latter uses the model approach to add persons. Thus, both ways are tested. Search for id is not that important, as this should not be done as a typical workflow. These ids are not consistent among database installations (so your persons receive other ids than mine).

So, to conclude: While I think there is always room for enhancing tests, I think we should rather focus on add missing ones and develop the code, than on improving old unittests. I think we can close this issue. If you belive, we need more dicussion on that, feel free to reopen it.