Проходит ли Code Review плохой код покрытый тестами

Плохой код с тестами должен пройти Code Review?

Нет, плохой код не должен проходить Code Review, даже если он полностью покрыт тестами. Code Review — это процесс контроля качества, который охватывает гораздо больше аспектов, чем просто проверка наличия тестов. Его цель — улучшить код и защитить долгосрочное здоровье кодовой базы, а не механически пропускать всё, что «зелёное».

Почему тесты — это не панацея

Наличие тестов, даже с высоким coverage, — это лишь один из критериев. Тесты проверяют что код делает, но не оценивают как он это делает. Плохой код с тестами подобен дому, который стоит, но имеет кривые стены, протекающую крышу и неудобную планировку. Он может функционировать, но создаёт массу проблем в будущем.

Основные цели Code Review, которые выходят за рамки проверки тестов:

• Читаемость и поддерживаемость: Код должен быть понятен другим разработчикам.
• Архитектурная целостность: Соответствие проекту, использование правильных паттернов.
• Производительность: Отсутствие неочевидных узких мест.
• Безопасность: Отсутствие уязвимостей.
• Масштабируемость: Возможность развития кода без переписывания.

Каким может быть «плохой код с тестами»?

Давайте рассмотрим конкретные примеры.

Пример 1: Нарушение принципов SOLID и читаемости

Представьте функцию, которая «работает» и покрыта юнит-тестами, но является монолитом и нарушает SRP (Принцип единственной ответственности).

// ПЛОХО: Смешивает логику форматирования, вычислений и валидации.
function processUserData(users) {
  const result = [];
  for (let i = 0; i < users.length; i++) {
    // Валидация
    if (!users[i].name || users[i].age < 0) {
      throw new Error('Invalid user data');
    }
    // Логика
    let status = 'basic';
    if (users[i].age > 65) {
      status = 'senior';
    } else if (users[i].age > 18) {
      status = 'adult';
    }
    // Форматирование
    result.push({
      fullName: users[i].name.toUpperCase(),
      userAge: users[i].age,
      category: status,
      discountEligible: status === 'senior'
    });
  }
  return result;
}

// Тест может быть зелёным, но код ужасен.
test('processUserData returns correct array', () => {
  const input = [{name: 'John', age: 30}];
  const output = processUserData(input);
  expect(output[0].category).toBe('adult');
});

На Code Review такому коду будет справедливо отказано. Рецензент попросит разбить функцию на отдельные (validateUser, determineUserStatus, formatUserData), что улучшит тестируемость и читаемость каждой части.

Пример 2: Неоптимальные алгоритмы и скрытые проблемы

// ПЛОХО: Квадратичная сложность O(n²) там, где можно O(n).
function findDuplicates(arr) {
  const duplicates = [];
  for (let i = 0; i < arr.length; i++) {
    for (let j = i + 1; j < arr.length; j++) {
      if (arr[i] === arr[j] && !duplicates.includes(arr[i])) {
        duplicates.push(arr[i]);
      }
    }
  }
  return duplicates;
}

// Тест пройдёт на малом наборе данных, но в продакшене упадёт производительность.
test('finds duplicates', () => {
  expect(findDuplicates([1, 2, 3, 2, 4, 1])).toEqual([2, 1]);
});

Рецензент должен указать на неэффективность и предложить использовать Set для решения за линейное время.

Роль тестов в Code Review

Тесты в таком контексте — это мощный инструмент для рецензента, а не индульгенция для автора.

• Доверие к рефакторингу: Если код плох, но покрыт тестами, у разработчика есть безопасная сетка для его переделки. Рецензент может сказать: «Исправь архитектуру, у тебя есть тесты, которые проверят, не сломал ли ты логику».
• Качество самих тестов: Code Review проверяет и тесты. Плохие тесты (хрупкие, зависимые от реализации, не проверяющие граничные случаи) — это тоже часть «плохого кода».
• Индикатор проблем: Если для простой функциональности написана уйма запутанных тестов, это может быть сигналом о чрезмерной сложности самого кода.

Правильный подход в процессе Review

1. Автор: Не должен использовать тесты как аргумент против правок. «Работает» — недостаточно. Нужно стремиться к «работает хорошо».
2. Рецензент: Должен давать конструктивную обратную связь, ссылаясь на конкретные принципы (DRY, KISS, SOLID), гайдлайны проекта или потенциальные риски. Комментарий «это плохо» бесполезен. Нужно: «Этот метод делает три разных вещи, давай разделим его, так будет проще тестировать и изменять каждую часть по отдельности. Вот пример...».
3. Команда: Должна иметь согласованные Definition of Done (DoD), куда входят не только проходящие тесты, но и соблюдение стандартов кодирования, отсутствие дублирования, приемлемая сложность и положительный отзыв от коллег.

Итог: Code Review — это страховка от технического долга. Пропуская плохой, но покрытый тестами код, команда сознательно берёт на себя этот долг, который будет накапливать проценты в виде сложности поддержки, багов и снижения скорости разработки. Хорошие тесты облегчают жизнь, позволяя смело рефакторить плохой код, но они не являются оправданием для его существования.