Code review для QA

Updated Jul 2026

Ревью тестового кода: другая оптика

Ревью тестового кода отличается от ревью кода приложения. Ревью кода приложения фокусируется на корректности, производительности и поддерживаемости. Ревью тестового кода фокусируется на детерминизме, изоляции, понятности и содержательных утверждениях. Тест, который проходит сегодня, но случайно падает завтра, хуже, чем отсутствие теста.

Чеклист QA-ревью

При ревью тестовых PR других инженеров оценивайте каждый тест по следующим критериям:

1. Детерминизм

Даёт ли тест одинаковый результат каждый раз, независимо от того, когда и где он запускается?

Тревожные признаки:

• Зависимость от текущей даты или времени (new Date())
• Зависимость от конкретного состояния базы данных, которое другие тесты могут изменить
• Использование Math.random() или других недетерминированных входных данных без seed
• Ожидание фиксированных интервалов (sleep(3000)) вместо динамических условий
• Зависимость от порядка элементов в множестве или неупорядоченной коллекции

// BAD: Depends on current time
test('show greeting', async () => {
  await page.goto('/');
  // This test fails after 6 PM
  await expect(page.locator('.greeting')).toHaveText('Good morning');
});

// GOOD: Mock the time
test('show morning greeting before noon', async () => {
  await page.clock.setFixedTime(new Date('2024-06-15T09:00:00'));
  await page.goto('/');
  await expect(page.locator('.greeting')).toHaveText('Good morning');
});

2. Независимость

Может ли этот тест запускаться изолированно, или он зависит от предварительного выполнения других тестов?

Тревожные признаки:

• Тест B предполагает, что тест A создал пользователя в базе данных
• Тесты используют общую глобальную переменную, которая модифицируется
• Порядок тестов имеет значение (падает при запуске с --randomize-order)
• Очистка происходит в «последнем» тесте, а не в afterEach/afterAll

// BAD: Depends on previous test
test('login with created user', async () => {
  // Assumes "create user" test ran first and created "testuser@example.com"
  await loginAs('testuser@example.com');
});

// GOOD: Each test creates its own data
test('login with valid credentials', async () => {
  const user = await createTestUser({ email: 'login-test@example.com' });
  await loginAs(user.email, user.password);
  await expect(page).toHaveURL('/dashboard');
  await deleteTestUser(user.id); // Cleanup
});

3. Содержательные утверждения

Достаточно ли утверждения конкретны, чтобы ловить реальные баги, но не настолько хрупки, чтобы ломаться при несущественных изменениях?

Тревожные признаки:

• Утверждения на точные пиксельные позиции или скриншотные совпадения без допуска
• Проверка только того, что страница загрузилась (без проверки конкретного содержимого)
• Утверждения на детали реализации (внутренние имена классов, data-атрибуты, которые могут измениться)
• Отсутствие утверждений (тест выполняет действия, но никогда не проверяет результаты)

// BAD: Too brittle -- breaks when any text changes
await expect(page.locator('.cart')).toHaveText(
  'Your cart contains 3 items totaling $59.97 including tax'
);

// BAD: Too loose -- passes even when the feature is broken
await expect(page.locator('.cart')).toBeVisible();

// GOOD: Specific enough to catch bugs, stable enough to survive minor changes
await expect(page.locator('.cart-count')).toHaveText('3');
await expect(page.locator('.cart-total')).toContainText('$59.97');

4. Именование

Можно ли понять, что проверяет тест, только по его названию?

Тревожные признаки:

• test1, test2, test_new
• Названия описывают, что тест делает, а не что проверяет
• Отсутствие блоков describe для группировки связанных тестов

// BAD: Meaningless names
test('test1', async () => { /* ... */ });
test('checkout works', async () => { /* ... */ });

// GOOD: Self-documenting names
describe('checkout', () => {
  test('displays order summary with correct item count and total', async () => { /* ... */ });
  test('shows validation error when credit card is expired', async () => { /* ... */ });
  test('redirects to confirmation page after successful payment', async () => { /* ... */ });
});

5. Очистка

Выполняет ли тест за собой очистку?

Тревожные признаки:

• Тест создаёт пользователей, заказы или файлы, но никогда их не удаляет
• Состояние браузера (cookies, localStorage) перетекает между тестами
• Временные файлы накапливаются и со временем вызывают проблемы с дисковым пространством

// GOOD: Explicit cleanup
let testUser: User;

test.beforeEach(async () => {
  testUser = await createTestUser();
});

test.afterEach(async () => {
  await deleteTestUser(testUser.id);
});

Конструктивная обратная связь

Цель code review — улучшить код, а не продемонстрировать своё превосходство. Формулируйте обратную связь как вопросы или предложения, а не приказы.

Плохая обратная связь

"Это неправильно."
"Зачем вы сделали это так?"
"Это никогда не будет работать."

Хорошая обратная связь

"Вы не рассматривали использование селектора data-testid здесь? По моему
опыту, селекторы по CSS-классам склонны ломаться, когда дизайн-команда
обновляет стили."

"Я думаю, это утверждение может быть нестабильным, потому что зависит от
тайминга анимации. Что если мы сначала подождём стабильности элемента?"

"Отличный подход! Одна мысль: можем ли мы выделить эту настройку в фикстуру,
чтобы другие тесты могли её переиспользовать? Я вижу похожий паттерн в
checkout.spec.ts."

Калибровка обратной связи

Одобряйте с незначительными комментариями, а не блокируйте из-за стилистических предпочтений. Оставляйте «запрос изменений» для проблем, которые вызовут реальные неприятности (нестабильность, отсутствие утверждений, проблемы безопасности).

На что обращать внимание в PR без тестов (как QA-инженеру)

QA-инженеры должны также ревьюировать PR с кодом приложения через призму тестируемости:

• Можно ли изменение протестировать? Есть ли хуки (data-testid атрибуты, API-контракты), делающие тестирование простым?
• Есть ли новые состояния ошибок? Новые пути кода означают новые тест-кейсы.
• Изменяет ли это существующее поведение? Возможно, нужно обновить существующие тесты.
• Есть ли миграции базы данных? Изменения схемы могут сломать существующие тестовые данные.
• Достаточна ли обработка ошибок? Отсутствие обработки ошибок создаёт нетестируемые режимы сбоев.

// In a review, you might comment:
// "This new endpoint doesn't return a meaningful error for invalid input.
// Could we return a 400 with a JSON body? That would make it much easier
// to write specific assertions in our API tests."

Советы по рабочему процессу ревью

Используйте предложенные изменения

Функция «suggest changes» в GitHub позволяет предлагать конкретные правки кода, которые автор может принять одним кликом:

// Instead of fixed wait, use a dynamic condition
await page.waitForSelector('.results', { state: 'visible' });

Группируйте комментарии

Прочитайте весь PR перед оставлением комментариев. Ваш комментарий к строке 10 может быть отвечен кодом на строке 200. Соберите все комментарии и отправьте их вместе.

Быстро проводите повторное ревью

Когда автор учёл вашу обратную связь, проведите повторное ревью оперативно. Цикл ревью длительностью 3 дня на каждом раунде замедляет всех.

Сначала проведите ревью своих собственных PR

Перед запросом ревью прочитайте свой дифф, как если бы вы были ревьюером. Вы часто обнаружите:

• Отладочный код, оставленный в коде (console.log, test.only)
• Пропущенные тест-кейсы для граничных сценариев
• Неясные имена переменных
• Случайные изменения файлов (конфигурация редактора, изменения lock-файла)

Практическое упражнение

1. Возьмите недавний тестовый PR в вашем репозитории и проведите ревью по пятиточечному чеклисту (детерминизм, независимость, утверждения, именование, очистка)
2. Потренируйтесь в написании обратной связи каждого уровня серьёзности (блокирующая, предложение, мелочь, вопрос, похвала)
3. Проведите ревью PR с кодом приложения через призму тестируемости. Какие новые тесты потребуются?
4. Настройте ротацию ревью, при которой QA-инженеры еженедельно проводят перекрёстное ревью тестовых PR друг друга
5. Создайте командный чеклист ревью, специфичный для паттернов тестирования вашего проекта, и опубликуйте его в командной wiki