eShopOnWeb Architecture (14/16) – uses parameterized tests

I love parameterized tests.

Instead of adding many different test methods with different inputs set in the method body we have one method and pass in the inputs and expected output(s) for each test we want to run as params to the method (one per line). This means less test methods (so test logic easier to change) and that we can quickly see all inputs and expected outputs for a particular method together. In addition to this our results from each row of params will be grouped together.

Below is one example from eShopOnWeb which uses xUnit, but most test frameworks will support the concept. Click on the image for a larger view in a new tab…

In this particular test the query inside the specification is being tested.. for me this is a little bit too low level of a test. There’s very little I can refactor in the specification without changing its signature and thus requiring a change to the test so personally I would test this stuff but would do it through the context of its parent API or use case which appears to be…

CatalogViewModelService.cs -> GetCatalogItems
CachedCatalogViewModelService.cs -> GetCatalogItems

eShopOnWeb Architecture (10/16) – has unit tests which test very low level implementation details

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.