Как уже упоминалось, вам, вероятно, не нужны поля user, shoppingList и meals в вашем сервисе.Эти поля делают службу небезопасной для использования в многопоточной среде, такой как веб-приложение или веб-служба (к которой могут обращаться несколько клиентов, то есть несколько потоков одновременно).Например, shoppingList, над которым вы работаете, может быть очищено в середине процесса, если другой поток введет createShoppingList.Вместо этого сделайте эти поля локальными переменными внутри метода createShoppingList.Если логика становится слишком сложной, а ваша служба слишком большой, вы можете извлечь ее в отдельный сервис или вспомогательный класс, который создается в начале вызова метода и отбрасывается в конце.
Я всегданаписать модульные тесты как тесты белого ящика для одного класса.Я стараюсь охватить каждую ветку в коде, если смогу.Вы можете проверить это, запустив тесты с покрытием в IntelliJ.Обратите внимание, что тесты черного ящика также очень полезны, они сосредоточены на «контракте» компонента.По моему мнению, модульные тесты обычно не подходят для этого, поскольку контракт одного класса обычно не очень интересен для функциональности компонента в целом и может легко измениться, если код подвергается рефакторингу.Я пишу интеграционные (или сквозные) тесты как тесты черного ящика.Это требует настройки среды приложения-заглушки, например, с базой данных в памяти и, возможно, с некоторыми внешними службами через WireMock.Если вы заинтересованы в этом, изучите условия контрактного тестирования Google или среду RestAssured.
Некоторые замечания по поводу вашего кода:
public Map<Ingredient,Long> createShoppingList() {
// if any of the chained methods below return null, a NullPointerException occurs
// You could extract a method which takes the userInfoService user as an argument, see `findUser` below.
user = userRepository.findByLoginAndPassword(userInfoService.getUser().getLogin(),userInfoService.getUser().getPassword()).get();
// the above would then become:
User user = findUser(userInfoService.getUser()).orElseThrow(new ShoppingServiceException("User not found");
// instead of clearing these field, just initialize them as local variables:
shoppingList.clear();
meals.clear();
meals = user.getDiet().getMeals();
// I would change adjustIngredients so it doesn't return the meals but void
// it's expected that such a method modifies the meals without making a copy
meals = dietMealsService.adjustIngredients(meals);
// I would extract the below iteration into a separate method for clarity
for (MealInfo meal : meals) {
// I would also extract the processing of a single meal into a separate method
// the `meal.getIngredients` actually doesn't return Ingredients but IngredientWeights
// this is very confusing, I would rename the field to `ingredientWeights`
meal.getMeal().getIngredients().forEach(s -> {
// I would replace the four calls to s.getIngredient() with one call and a local variable
// and probably extract another method here
// You are using Ingredient as the key of a Map so you must implement
// `equals` and // `hashCode`. Otherwise you will be in for nasty
// surprises later when Java doesn't see your identical ingredients as
// equal. The simplest would be to use the database ID to determine equality.
if(shoppingList.containsKey(s.getIngredient()))
shoppingList.put(s.getIngredient(), s.getWeight()+shoppingList.get(s.getIngredient()));
else
shoppingList.put(s.getIngredient(),s.getWeight());
});
}
return shoppingList;
}
private Optional<User> findUser(my.service.User user) {
if (user != null) {
return userRepository.findByLoginAndPassword(user.getLogin(), user.getPassword());
}
else {
return Optional.empty();
}
}
private void processMeals(List<MealInfo> meals, Map<Ingredient, Long> shoppingList) {
for (MealInfo mealInfo : meals) {
processIngredientWeights(mealInfo.getMeal().getIngredients(), shoppingList);
}
}
private void processIngredientWeights(List<IngredientWeight> ingredientWeights, Map<Ingredient, Long> shoppingList) {
for (IngredientWeight ingredientWeight: ingredientWeights) {
processIngredientWeight(ingredientWeight, shoppingList);
}
}
private void processIngredientWeight(IngredientWeight ingredientWeight, Map<Ingredient, Long> shoppingList) {
Ingredient ingredient = ingredientWeight.getIngredient();
Long weight = shoppingList.getOrDefault(ingredient, 0L);
weight += ingredientWeight.getWeight();
shoppingList.put(ingredient, weight);
}
РЕДАКТИРОВАТЬ: я снова посмотрел на ваш код и домен и сделал несколькоизменения, см. мой пример кода здесь: https://github.com/akoster/x-converter/blob/master/src/main/java/xcon/stackoverflow/shopping
Модель предметной области была немного запутанной из-за классов 'Info'.Я переименовал их следующим образом:
MealInfo -> Meal
Meal -> Recipe (with a list of Ingredients)
IngredientInfo -> Ingredient (represents a certain amount of a FoodItem)
Ingredient -> FoodItem (e.g. 'broccoli')
Я понял, что служба не принимает аргументов!Это немного странно.Имеет смысл получить пользователя отдельно (например, в зависимости от текущего / выбранного пользователя) и передать его в службу, как вы видите выше.Теперь ShoppingListService выглядит следующим образом:
public class ShoppingListService {
private DietMealsService dietMealsService;
public ShoppingListService(DietMealsService dietMealsService) {
this.dietMealsService = dietMealsService;
}
public ShoppingList createShoppingList(User user) {
List<Meal> meals = getMeals(user);
dietMealsService.adjustIngredients(meals);
return createShoppingList(meals);
}
private List<Meal> getMeals(User user) {
Diet diet = user.getDiet();
if (diet == null || diet.getMeals() == null || diet.getMeals().isEmpty()) {
throw new ShoppingServiceException("User doesn't have diet");
}
return diet.getMeals();
}
private ShoppingList createShoppingList(List<Meal> meals) {
ShoppingList shoppingList = new ShoppingList();
for (Meal meal : meals) {
processIngredientWeights(meal.getRecipe().getIngredients(), shoppingList);
}
return shoppingList;
}
private void processIngredientWeights(List<Ingredient> ingredients, ShoppingList shoppingList) {
for (Ingredient ingredient : ingredients) {
shoppingList.addWeight(ingredient);
}
}
}
Я также ввел класс ShoppingList, потому что передача Map вокруг - это запах кода, и теперь я могу переместить логику, чтобы добавить ингредиент в список покупок.класс.
import lombok.Data;
@Data
public class ShoppingList {
private final Map<FoodItem, Long> ingredientWeights = new HashMap<>();
public void addWeight(Ingredient ingredient) {
FoodItem foodItem = ingredient.getFoodItem();
Long weight = ingredientWeights.getOrDefault(foodItem, 0L);
weight += ingredient.getWeight();
ingredientWeights.put(foodItem, weight);
}
}
Модульный тест для этой службы теперь выглядит следующим образом:
@RunWith(MockitoJUnitRunner.class)
public class ShoppingListServiceTest {
@InjectMocks
private ShoppingListService instanceUnderTest;
@Mock
private DietMealsService dietMealsService;
@Mock
private User user;
@Mock
private Diet diet;
@Mock
private Meal meal;
@Test(expected = ShoppingServiceException.class)
public void testCreateShoppingListUserDietNull() {
// SETUP
User user = mock(User.class);
when(user.getDiet()).thenReturn(null);
// CALL
instanceUnderTest.createShoppingList(user);
}
@Test(expected = ShoppingServiceException.class)
public void testCreateShoppingListUserDietMealsNull() {
// SETUP
when(user.getDiet()).thenReturn(diet);
when(diet.getMeals()).thenReturn(null);
// CALL
instanceUnderTest.createShoppingList(user);
}
@Test(expected = ShoppingServiceException.class)
public void testCreateShoppingListUserDietMealsEmpty() {
// SETUP
when(user.getDiet()).thenReturn(diet);
List<Meal> meals = new ArrayList<>();
when(diet.getMeals()).thenReturn(meals);
// CALL
instanceUnderTest.createShoppingList(user);
}
@Test
public void testCreateShoppingListAdjustsIngredients() {
// SETUP
when(user.getDiet()).thenReturn(diet);
List<Meal> meals = Collections.singletonList(meal);
when(diet.getMeals()).thenReturn(meals);
// CALL
instanceUnderTest.createShoppingList(user);
// VERIFY
verify(dietMealsService).adjustIngredients(meals);
}
@Test
public void testCreateShoppingListAddsWeights() {
// SETUP
when(user.getDiet()).thenReturn(diet);
when(diet.getMeals()).thenReturn(Collections.singletonList(meal));
Recipe recipe = mock(Recipe.class);
when(meal.getRecipe()).thenReturn(recipe);
Ingredient ingredient1 = mock(Ingredient.class);
Ingredient ingredient2 = mock(Ingredient.class);
when(recipe.getIngredients()).thenReturn(Arrays.asList(ingredient1, ingredient2));
FoodItem foodItem = mock(FoodItem.class);
when(ingredient1.getFoodItem()).thenReturn(foodItem);
when(ingredient2.getFoodItem()).thenReturn(foodItem);
Long weight1 = 42L;
Long weight2 = 1337L;
when(ingredient1.getWeight()).thenReturn(weight1);
when(ingredient2.getWeight()).thenReturn(weight2);
// CALL
ShoppingList shoppingList = instanceUnderTest.createShoppingList(user);
// VERIFY
Long expectedWeight = weight1 + weight2;
Long actualWeight = shoppingList.getIngredientWeights().get(foodItem);
assertEquals(expectedWeight, actualWeight);
}
}
Надеюсь, это довольно очевидно.
Кстати, помните, что юнитТест должен проверять только тестируемый класс.Постарайтесь свести к минимуму любые предположения о поведении других классов и сделать это явным, высмеивая их, как показано выше.По той же причине я всегда стараюсь избегать использования «реалистичных» тестовых данных в модульных тестах, потому что это говорит о том, что значения имеют значение для теста - они не имеют.