Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Develop1 #924

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Develop1 #924

wants to merge 10 commits into from

Conversation

kotovserge
Copy link

No description provided.


@Test
public void getNameBunTest() {
Assert.assertEquals("Флюоресцентная булка R2-D3", bun.getName());

Choose a reason for hiding this comment

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

⚠️Можно улучшить. Можно использовать поясняющие сообщения для assert. Тогда будет проще разобраться, почему тест упал.

Copy link
Author

Choose a reason for hiding this comment

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

Добавил поясняющие сообщения для assert.


@Before
public void init() {
bun = new Bun("Флюоресцентная булка R2-D3", Float.valueOf(998));

Choose a reason for hiding this comment

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

⛔️Нужно исправить. Ты проверила позитивные тесты для цены и названия булочки. Это хорошо. Но давай применим метод КЭ и ГЗ и проверим хотя бы основные КЭ. Например, для цены -- отрицательная цена, ноль, положительная цена (дробная)

Copy link
Author

Choose a reason for hiding this comment

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

Добавил проверки.

private String receipt;

@Spy
Burger burger;

Choose a reason for hiding this comment

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

⛔️Нужно исправить. Создавай моки только для зависимостей, а не для тестируемого объекта. Bun и Ingredient должны быть моками

Copy link
Author

Choose a reason for hiding this comment

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

Использовал моки для Bun и Ingredient.

bun = new Bun(BUN_NAME, BUN_PRICE);
ingredientTypeSauce = IngredientType.SAUCE;
ingredientTypeFilling = IngredientType.FILLING;
Ingredient ingredientSauce = new Ingredient(ingredientTypeSauce, SAUSE_NAME, SAUSE_PRICE);

Choose a reason for hiding this comment

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

⛔️Нужно исправить. Используй моки, а не реальные объекты.

Copy link
Author

Choose a reason for hiding this comment

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

После добавления моков для Bun и Ingredient использовал их, вместо реальных объектов.

// Собираем бургер
// Добавляем булку
burger.setBuns(bun);
Mockito.verify(burger).setBuns(bun);

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

Каждую проверку вынес в отдельный тест.

}

@Parameterized.Parameters
public static Object[][] setParams() {

Choose a reason for hiding this comment

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

⚠️Можно улучшить. В параметризованных тестах для аннотации Parameterized.Parameters лучше использовать аргумент name: @Parameterized.Parameters(name = "Тестовые данные: {0} {1}"), где {0}, {1} - индексы параметров. Это повысит информативность проверки

Copy link
Author

Choose a reason for hiding this comment

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

Добавил аргумент name в аннотацию Parameterized.Parameters

Choose a reason for hiding this comment

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

⛔️Нужно исправить. Не проверен enum IngredientType

Copy link
Author

Choose a reason for hiding this comment

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

Добавил проверку для enum IngredientType.

burger.addIngredient(ingredientFilling);
burger.addIngredient(ingredientFilling);
// Удаляем ингредиенты
burger.removeIngredient(1);

Choose a reason for hiding this comment

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

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

  • На удаление ингредиента - отдельные тест
  • На добавление ингредиента - отдельные тест итд

Copy link
Author

Choose a reason for hiding this comment

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

Исправил

}

@Test
public void test(){

Choose a reason for hiding this comment

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

⛔️Нужно исправить. Неинформативный заголовок

Copy link
Author

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