Does Code Review Involve Testing?

The following text is a partial translation of the original English article, performed by ChatGPT (gpt-3.5-turbo) and this Jekyll plugin:

Когда вы рассматриваете запрос на объединение от кого-то, вы проверяете ветку и запускаете сборку? Я обычно этого не делаю, но некоторые люди делают. Их очевидная причина заключается в том, что выполнение сборки или даже ручное тестирование продукта помогает находить более важные ошибки. Простое просмотрение исходного кода может не выявить все визуальные дефекты, которые были недавно внесены в HTML/CSS, например. Лучше проверить ветку, запустить Apache, открыть сайт в Chrome и посмотреть, что сломано. Затем сделать снимок экрана, прикрепить его к запросу на объединение и вернуть его автору. Но я не согласен с этим, и вот почему.

Эта дискуссия не нова, посмотрите это и это на SO. Однако кажется, что все ответы там упускают главный момент.

Как мы все знаем, в любом программном проекте есть две противоположные роли: конструкторы и деструкторы, также известные как программисты и тестировщики. Программисты добавляют новые функции и исправляют ошибки. Их результат - количество созданного функционала: чем больше, тем лучше. В то время как тестировщики ломают продукт и сообщают об ошибках, делая все возможное, чтобы доказать, что он еще не готов к отправке заказчику. В какой-то момент команда (или руководство) решает, что битва окончена, и продукт может быть доставлен.

Благодаря этому противоречию достигается качество.

Программисты заканчивают свою часть битвы, когда код проходит процесс слияния: изменения вносятся на их ноутбуках, модульные тесты проходят локально, статический анализ не жалуется, сборка чистая, и ветка объединяется с основной. Здесь программисты останавливаются и получают свои бонусы.

Тестировщики заканчивают свою часть битвы, когда им удается найти новый дефект в продукте, развернутом на стадии тестирования или в производственной среде: баг найден, сообщен и принят проектом. Здесь тестировщики останавливаются и получают свои бонусы.

Это очевидно. Если это не так, вам может захотеться прочитать Искусство тестирования программного обеспечения Гленфорда Майерса или Code Ahead Йегора Бугаенко. Вы также можете посмотреть это видео.

Теперь, где место рецензента кода в этом противоречии?

Я считаю, что рецензия кода является частью процесса слияния, вместе с модульными тестами, статическими анализаторами, линтерами, инструментами контроля покрытия, тестировщиками мутаций и всем остальным, что проект может захотеть поместить туда, чтобы сделать жизнь программистов сложнее и качество исходного кода выше. Цель процесса слияния - защитить репозиторий кода от программистов.

Рецензенты кода ничем не отличаются от линтеров или статических анализаторов: они блокируют ветки с плохим кодом, не позволяя им попасть в основную ветку. Когда они блокируют, они указывают причины. Их работа (подобно линтерам) заканчивается, когда им удается найти проблему в ветке и объяснить ее. Линтер, отклоняя запрос на объединение, точно указывает, где проблема, на какой строке и как ее можно исправить. Именно это должен делать хороший рецензент кода: находить проблемные строки, объяснять проблему и предлагать решение.

За это рецензенты кода должны получать оплату: завершенные рецензии.

Что такое завершенная рецензия? Звучит ли “Все в порядке” как завершенная рецензия? Для линтера - да; для рецензента кода - нет. Гораздо лучше звучит такое описание: “Я нашел три проблемы, объяснил их и они либо были обсуждены, либо исправлены”. Так может звучать описание работы рецензента кода: Найти три самые критические проблемы, объяснить их и убедиться, что они либо исправлены, либо правильно обоснованы.

Как рецензенты находят эти три проблемы - это уже их выбор. Они могут проводить визуальный осмотр кода или запускать сборку. Однако они все равно будут получать оплату, когда удастся найти проблемы и убедиться, что автор кода их понимает и либо исправляет, либо объясняет, почему их нельзя исправить (или почему это не проблемы). Визуальный осмотр кода происходит быстро, в то время как проверка ветки и запуск сборки являются более трудоемкими операциями. Более того, ошибку, обнаруженную после запуска сборки локально, очень трудно объяснить в формате рецензии кода. Обсуждение с автором ветки займет больше времени, что означает больше времени для завершения рецензии кода, что в свою очередь приведет к снижению эффективности рецензента кода.

Мой посыл в том, что умные рецензенты кода не делают этого, потому что это неэффективно. Обратите внимание, не потому, что им все равно, а потому, что они знают более эффективный способ внести свой вклад в проект. Давайте признаем, что когда ветка перед нами проходит все автоматические проверки, такие как линтеры и модульные тесты, и все же имеет некоторые ошибки, которые мы можем воспроизвести только выполнив код, что-то не так с нашими автоматическими тестами. Эффективный, ответственный и алчный рецензент кода не будет объяснять автору кода, что именно не так. Вместо этого будет создана новая ошибка, чтобы обвинить процесс слияния в недостаточной надежности. И, конечно же, эта новая ошибка будет вознаграждена.

Таким образом, в качестве рецензента кода вы можете работать с веткой локально, тестировать ее и сообщать автору свои результаты. Но это будет противоречить вашим личным интересам и не будет полезным для проекта. Вместо этого вам следует пожаловаться на низкое качество автоматических тестов проекта и приостановить рецензию. Когда жалоба будет рассмотрена, тесты станут сильнее, вы вернетесь к рецензии, которая будет отклонена процессом слияния, а не вами.

В этом случае все выигрывают: процесс слияния становится сильнее, вы получаете дополнительный бонус за сообщенный дефект, а рецензия отклоняется с очень конкретной воспроизводимой причиной.

P.S. Идея этой статьи в блоге была предложена Робертом Зёземанном.

Translated by ChatGPT gpt-3.5-turbo/42 on 2023-11-17 at 17:05

sixnines availability badge   GitHub stars