search

britter / bootify-testpyramid

Apache License 2.0
5 stars 2 forks source link

Some small improvements #1

Closed mvitz closed 7 years ago

mvitz commented 7 years ago

This PR contains some small stuff that may be an improvement. Feel free to pick only some commits if you have another opinion.

Regarding the Unit tests: I prefer test data builders overt he template approach. E.g. in OrderTest I find it hard to see why each order results in the asserted price. With builders the link between input and asserted output is sometimes a little bit more clear. Maybe this specific example should not test the price calculation at all because it's not the responsibility of an order and is already tested in the PriceCalculation.

Regarding the system tests: I personally prefer using a real HTTP client instead of MockMvc to hit the real endpoint and not start within the spring layer.

britter commented 7 years ago

Time to set up Travis CI et al. :-)

This changes look good to me. I learned something today, which is awesome! Thank you!

Regarding your comment about Unit tests: I agree, it is not obvious why that price was calculated. My idea is to have a common understanding oh what a smallOrder etc. is. So everybody should know what the delivery of a small order costs. So I prefer to use my templates here. The other side is the expected result. Maybe is is more explicit to reference the FIXED_PRICE constant for example. This way the code would change like this:

assertThat(smallOrder().calculateDeliveryPrice()).isEqualTo(FIXED_PRICE)

I'm not sure about this an need to think more about it.

Regarding your comment about system tests: I got this feedback after my session and would like to see how that looks like. Would you like to show this in a PR?