eShopOnWeb has some really low level tests which IMHO provide low or no value in terms of being able to guard against regression bugs.
For example in the CacheHelper class shown below there’s a static method called GenerateBrandsCacheKey which returns “brands” and in the UnitTest project there’s a corresponding test which checks for this exact value.
Click the image for a larger view in a new window…
This test is pointless. If I change “brands” to “brandsABC” in the helper method, the test will fail… but to make that test pass again I have to change the test, not the code under test. Normally (unless we are doing something other than refactoring) when we having a failing test, we fix the application code to go green again, not the test. The test should not need to change.
Also from a functional perspective, what value does confirming that these helper methods return specific strings have? IMHO there is none as the specific strings have no meaning in regards to the caching logic. All that matters is that individual items are stored and retrieved from the cache using the same string… the specific value of that string is not important…
Personally I’d delete these three cache helper tests and just focus on testing at a higher level such as the GetBrands() method above. To be honest I’d probably delete the CacheHelpers class altogether and just put the keys etc. into the CachedCatalogViewModelService class itself.
What do you think? … Keep in mind that eShopOnWeb is reference architecture so perhaps these tests are just for demonstration purposes.
