Skip to content

Review#1

Open
Freezebee22 wants to merge 3 commits intomasterfrom
review
Open

Review#1
Freezebee22 wants to merge 3 commits intomasterfrom
review

Conversation

@Freezebee22
Copy link
Owner

No description provided.

Copy link

@mikebars1995 mikebars1995 left a comment

Choose a reason for hiding this comment

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

Здравствуйте. (Нужно развернуть общий комментарий ↓)

Посмотрите на гитхабе все комментарии к коду (нужно прокрутить вниз страницу там)

Работа проделана огромная:

  • Все юнит-тесты (34 шт) проходят успешно
  • Все cypress-тесты проходят успешно
  • Код тестов аккуратный и понятный

но есть некоторые недочеты:

  • http://localhost:4000 обычно повторяется несколько раз в коде тестов. Нужно вынести в константу testUrl, чтобы можно было одним движением изменить урл для тестов.
  • Если строка-селектор повторяется более 2х раз в коде, значит, этот селектор нужно выносить в константу, чтобы можно было одним движением изменить его и не искать по всему коду дублирования этого селектора. Нужно исправить это для всех дублирований в тестах

beforeEach(() => {
cy.intercept('GET', 'api/ingredients', { fixture: 'ingredients.json' });

cy.visit('http://localhost:4000');

Choose a reason for hiding this comment

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

Нужно исправить

http://localhost:4000 обычно повторяется несколько раз в коде тестов. Нужно вынести в константу testUrl, чтобы можно было одним движением изменить урл для тестов.
А можно в документации посмотреть, как настроить быстро baseUrl для cypress https://docs.cypress.io/api/commands/visit#Visit-is-automatically-prefixed-with-baseUrl


describe('проверка закрытия модалок', () => {
it('крестик', () => {
cy.get('[data-cy="bun"]:first-of-type').click();

Choose a reason for hiding this comment

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

Нужно исправить

Если строка-селектор повторяется более 2х раз в коде, значит, этот селектор нужно выносить в константу, чтобы можно было одним движением изменить его и не искать по всему коду дублирования этого селектора. Нужно исправить это для всех дублирований в тестах

Можно лучше

  • Лучше использовать переменные alias в тестах, чтобы 1 раз найти элемент и обращаться к нему. Подробнее тут https://docs.cypress.io/api/commands/as#DOM-element
  • Попробуйте сделать команды cypress. Подробнее тут https://on.cypress.io/custom-commands
  • Воспользуйтесь лучшими практиками cypress. Подробнее

Copy link

@mikebars1995 mikebars1995 left a comment

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