Skip to content

Conversation

@avbolotova
Copy link

… Burger

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Папку .idea не нужно было загружать в репозиторий. Эта папка должна быть добавлена в .gitignore.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Папка из пулл реквеста не убрана


burger.addIngredient(extra);

assertEquals(before + 1, burger.ingredients.size());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Для юнит-тестов придерживаемся подхода: один тест, значит одна проверка. Если очень хочется несколько проверок -- тогда используем softAssertions. Поправь, пожалуйста, во всем коде

}

@Test
public void setBuns() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Если в тестах не нужна параметризация, их нужно вынести в отдельный класс

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. В папке target нужно оставить только отчет

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Папка из пулл реквеста не убрана


private Burger burger;
private Bun bunMock;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Должно быть два класса

  1. Класс с параметризированными тестами для burger. Я его ее вижу
  2. Класс с обычными тестами

import java.util.Arrays;
import java.util.Collections;

public class BurgerTestData {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️Можно улучшить. данные можно не отделять от тестов

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants