A Few Valid Reasons to Reject a Bug Fix

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

Существует ошибка, когда что-то не работает как ожидается. Исправление ошибки - это в основном патч (запрос на принятие изменений) для существующего кода, который предполагается решить проблему и убедиться, что “что-то” работает как ожидается. Очень часто такой патч исправляет одно, но ломает многое другое. Я считаю, что иногда необходимо отклонить исправление ошибки и попросить автора внести изменения в патч, чтобы защитить проект от более серьезных проблем. Согласно моему опыту, есть несколько действительных причин для такого отклонения.

Это очень распространенная ситуация: после внесения изменений в одном месте, модульные тесты не проходят в другом месте. Ошибка исправлена, но некоторые, возможно, не связанные с ней модульные тесты начинают сообщать о сбое. Под давлением или просто из-за лени мы не исправляем их; мы просто удаляем тесты или помечаем их как временно “пропущенные”. Проблема решена, сборка чистая, так что давайте объединим патч и закончим на сегодня, верно? Неверно!

Несмотря на то, что я за то, чтобы срезать углы насколько это возможно, это угол, который я не рекомендую вам срезать.

Модульные тесты существуют именно для того, чтобы предотвращать поломку продукта в условиях давления.

Очевидно, есть ситуации, когда модульные тесты ошибочны и их нужно удалить. В таких случаях не забудьте создать новые.

Также есть ситуации, когда ошибку нужно исправить за несколько минут, чтобы вернуть систему в онлайн, а исправление всех модульных тестов займет час. Такая ситуация является сильным показателем того, что у вас есть плохая ситуация с покрытием тестами продукта. Несомненно, мы должны исправить ошибку и попросить тесты замолчать на некоторое время. Но в этом случае убедитесь, что следующая задача, над которой работает ваша команда после исправления ошибки, - это исправление отключенных модульных тестов. Я рекомендую прочитать Working Effectively With Legacy Code Майкла Фезерса, где рассматривается именно эта тема.

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

Реальная проблема заключается в недостаточном покрытии кода тестами в этой конкретной части программы. Удаляя опечатку, мы никак не помогаем проекту. Более того, мы делаем ему плохую услугу - мы скрываем настоящую проблему.

Таким образом, независимо от того, насколько маленькой или косметической является проблема, исправление ошибки должно содержать дополнительный тест, который воспроизводит данную ошибку. Без такого теста исправление ошибки является пустой тратой денег проекта.

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

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

Никакого рефакторинга при исправлении ошибки!

Создайте новый модульный тест, воспроизведите ошибку и зафиксируйте ее. Исправьте ошибку в существующем коде, несмотря на его некрасивость. Создайте новые ошибки, попросите команду улучшить ситуацию с некрасивым кодом. Если заинтересованы, назначьте эти ошибки себе. Или, возможно, кто-то другой захочет исправить и переработать код. Но все это произойдет позже в других запросах на включение с новыми проверками кода и объединениями.

Здесь не речь идет о лени и неохоте исправлять то, что выглядит плохо. Речь идет о дисциплине, которая намного важнее хороших намерений.

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

Не важно, насколько простое исправление, держите его отдельно от других. Проверьте, протестируйте и объедините его индивидуально. Это также увеличит прослеживаемость изменений. Всегда будет легко понять, кто сделал это исправление, кто проверил код и когда оно было объединено (и развернуто).

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

sixnines availability badge   GitHub stars