valentinacupac / banking-kata-java

Banking Kata (Java)
MIT License
336 stars 90 forks source link

Adapter - Persistence - Redis - Consolidate mapper tests #90

Closed valentinacupac closed 1 year ago

valentinacupac commented 1 year ago

This ticket is about a theme that took me years to realize.

Several years ago, I used to do Mapper tests, well that was how many people did it too. The challenge here is that the mapper is an implementation detail, so then we're coupling our test to implementation. See meetup by @peterschuler https://www.youtube.com/watch?v=D5YSQkNFu_E

In general, when testing adapters, we want to test just the driver port interface, not the adapter implementation details. More specifically we're interested thus in just BankAccountStorage interface and don't care about how we implement it (don't care about how mapping is done, or whether or not you even have a mapper).

The file RedisBankAccountStorageTest extends BankAccountStorageTest. When opening BankAccountStorageTest we can see any test methods, for example the test method should_return_added_bank_account() creates some account, adds it to storage, and retrives it, verifies that it is equivalent to the added bank account. This serves as proof that any mapping (which is an implementation detail) works correctly - when we can retrieve back the same object that we had persisted. Thus, BankAccountStorageTest is already transitively testing mapping correctness; we do not need any additional mapping test.

Thus:

Thus from the purpose of the adapter, just test file BankAccountStorageTest is enough... (targeting BankAccountStorage) because that's the only "public" part of the module, everything else is internal to the module.

JoaoCipriano commented 1 year ago

Thank you for this excellent explanation @valentinacupac!