Это плохая практика, чтобы модифицировать код строго для целей тестирования

77

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

Мое мнение таково, что все в порядке, конечно, в рамках поддержания хороших объектно-ориентированных и программных практик (не «все обнародовать» и т. Д.).

Мое коллега считает, что изменение кода (который работает) только для целей тестирования является неправильным.

Просто простой пример, подумайте об этом фрагменте кода, который используется некоторым компонентом (написанным на C #):

public void DoSomethingOnAllTypes()
{
    var types = Assembly.GetExecutingAssembly().GetTypes();

    foreach (var currentType in types)
    {
        // do something with this type (e.g: read it's attributes, process, etc).
    }
}

Я предположил, что этот код можно изменить, чтобы вызвать другой метод, который будет выполнять реальную работу:

public void DoSomething(Assembly asm)
{
    // not relying on Assembly.GetExecutingAssembly() anymore...
}

Этот метод принимает объект Assembly для работы, позволяя передать вашу собственную Assembly для тестирования. Мой коллега не думал, что это хорошая практика.

Что считается хорошей и распространенной практикой?

liortal
источник
5
Ваш измененный метод должен принимать Typeв качестве параметра, а не Assembly.
Роберт Харви
Вы правы - тип или сборка, суть в том, что код должен позволять предоставлять это в качестве параметра, хотя может показаться, что эта функциональность предназначена только для тестирования ...
liortal
5
У вашего автомобиля есть порт ODBI для «тестирования» - если бы все было идеально, оно бы не понадобилось. Я предполагаю, что ваша машина более надежна, чем его программное обеспечение.
Mattnz
24
Какая у него уверенность в том, что код «работает»?
Крейг,
3
Конечно - сделай так. Протестированный код стабилен во времени. Но вам будет гораздо легче объяснить новые ошибки, если они будут улучшены, а не исправлены
Нордландер

Ответы:

135

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

  • Проще в обслуживании,
  • Легче рассуждать,
  • Более слабо связаны, и
  • Имеет лучший общий дизайн, архитектурно.
Роберт Харви
источник
3
Я не упомянул эти вещи в своем ответе, но я полностью согласен. Переработка вашего кода, чтобы сделать его более тестируемым, имеет несколько приятных побочных эффектов.
Джейсон Светт
2
+1: я вообще согласен. Существуют, безусловно, ясные случаи, когда тестирование кода наносит ущерб этим другим выгодным целям, и в этих случаях тестируемость является самым низким приоритетом. Но в целом, если ваш код недостаточно гибок / расширяем для тестирования, он не будет достаточно гибок / расширяем для использования или для изящного старения.
Теластин
6
Тестируемый код - лучший код. Однако всегда существует риск, связанный с модификацией кода, в котором отсутствуют тесты, и самый простой и дешевый способ уменьшить этот риск в чрезвычайно короткий срок - это оставить его в покое, что хорошо ... пока вам действительно не понадобится изменить код. Что вам нужно сделать, это продать преимущества модульного тестирования вашему коллеге; если все участвуют в модульном тестировании, то не может быть никаких аргументов в пользу того, что код должен быть тестируемым. Если не все участвуют в модульном тестировании, то нет смысла.
guysherman
3
Именно так. Помню, я писал модульные тесты для примерно 10 тысяч строк исходного кода моего собственного кода. Я знал, что код работал отлично, но тесты заставили меня переосмыслить некоторые ситуации. Я должен был спросить себя: «Что именно делает этот метод?». Я нашел несколько ошибок в рабочем коде, только взглянув на него с новой точки зрения.
Султан
2
Я продолжаю нажимать кнопку «вверх», но могу дать вам только одно очко. Мой нынешний работодатель слишком близорук, чтобы позволить нам на самом деле реализовывать модульные тесты, и я все еще извлекаю выгоду из осознанного написания своего кода, как будто я работаю на основе тестов.
AmericanUmlaut
59

В игре есть (казалось бы) противоборствующие силы .

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

Сторонники сохранения всех «подробностей реализации» обычно мотивируются желанием поддерживать инкапсуляцию. Однако хранение всего заблокированного и недоступного является неправильно понятым подходом к инкапсуляции. Если бы конечной целью было сохранение всего недоступного, единственно верный инкапсулированный код был бы следующим:

static void Main(string[] args)

Ваш коллега предлагает сделать это единственной точкой доступа в вашем коде? Должен ли весь другой код быть недоступным для внешних абонентов?

Едва. Тогда что может делать некоторые методы публичными? Разве это не субъективное дизайнерское решение?

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

Я бы не хотел подвергать частный метод , который не защищает его инварианты, но часто вы можете изменить его так , чтобы он действительно защищает ее инварианты, а затем выставить его на публике (конечно, с TDD, вы делаете это на наоборот).

Открытие API для тестируемости - это хорошо , потому что вы действительно применяете принцип Open / Closed .

Если у вас есть только один вызывающий пользователь вашего API, вы не знаете, насколько гибок ваш API на самом деле. Скорее всего, это довольно негибко. Тесты выступают в качестве второго клиента, предоставляя вам ценные отзывы о гибкости вашего API .

Таким образом, если тесты показывают, что вы должны открыть свой API, сделайте это; но поддерживать инкапсуляцию, не скрывая сложность, а раскрывая сложность безотказным способом.

Марк Симанн
источник
3
+1 Вы чувствуете себя в безопасности, выставляя публичный метод, когда он должным образом защищает свои инварианты.
Шамбулатор
1
Я думаю, ваш ответ! :) Сколько раз я слышу: «Инкапсуляция - это процесс, который делает приватными некоторые методы и свойства». Это все равно что говорить, когда вы программируете в объектно-ориентированном виде, это потому, что вы программируете с объектами. :( Я не удивлен, что такой чистый ответ исходит от мастера внедрения зависимостей. Я непременно прочту ваш ответ пару раз, чтобы заставить меня улыбнуться о моем плохом старом унаследованном коде.
Сэмюэль,
Я добавил несколько твиков в ваш ответ. Кроме того, я прочитал ваш пост « Инкапсуляция свойств» , и единственная убедительная причина, по которой я когда-либо слышал об использовании автоматических свойств, заключается в том, что изменение общедоступной переменной в публичное свойство нарушает бинарную совместимость; экспонирование его как автоматического свойства с самого начала позволяет добавить валидацию или другие внутренние функции, не нарушая клиентский код.
Роберт Харви
21

Похоже, вы говорите о внедрении зависимости . Это действительно часто встречается, и IMO довольно необходим для тестируемости.

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

Джейсон Светт
источник
DI - не главный вопрос (я только пытался привести пример), вопрос был в том, будет ли нормально предоставить другой метод, который изначально не предназначен для создания, только для тестирования.
liortal
4
Я знаю. Я думал, что обратился к вашему главному пункту во втором абзаце с «да».
Джейсон Светт
13

Это проблема курицы и яйца.

Одна из главных причин, почему хорошо иметь хорошее тестовое покрытие вашего кода, заключается в том, что он позволяет вам безбоязненно проводить рефакторинг. Но вы находитесь в ситуации, когда вам нужно провести рефакторинг кода, чтобы получить хорошее тестовое покрытие! А твой коллега боится.

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

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

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

Конечно, хорошая ли это деловая практика - это целая другая червь ..

Carson63000
источник
Важная проблема. Обычно я делаю рефакторинги, поддерживаемые IDE, потому что шанс сломать что-либо там очень низок. Если бы рефакторинг был более сложным, я бы сделал это только тогда, когда мне все равно нужно изменить код. Для того, чтобы изменить вещи, вам нужны тесты, чтобы снизить риски, чтобы вы даже получили ценность для бизнеса.
Ганс-Петер Стёрр
Дело в том, хотим ли мы сохранить старый код, потому что мы хотим передать ответственность за объект, запрашивающий текущую сборку, или потому, что мы не хотим добавлять некоторые открытые свойства или изменять сигнатуру метода? Я думаю, что код нарушает SRP, и по этой причине он должен быть реорганизован независимо от того, насколько боятся коллеги. Конечно, если это общедоступный API, используемый многими пользователями, вам придется подумать о такой стратегии, как реализация фасада или чего-либо, что поможет вам предоставить старому коду интерфейс, предотвращающий слишком много изменений.
Самуил
7

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

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

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

(Конечно, новый код активно поддерживается, поэтому он должен быть написан для тестирования.)

Эйдан Калли
источник
2

Я думаю, что ваш коллега не прав.

Другие уже упоминали причины, по которым это уже хорошо, но если вам дано это сделать, у вас все будет хорошо.

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

Не обязательно ваше место, чтобы принять решение о рефакторинге, а не работать над новыми функциями, которые принесут пользу вашей компании / клиента.

Ozz
источник
2

Я использовал инструменты покрытия кода как часть модульного тестирования при проверке всех ли путей через код. Будучи довольно хорошим программистом / тестировщиком самостоятельно, я обычно покрываю 80-90% путей кода.

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

Сэм Гаморан
источник
2

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

Многие люди изменили свой код для введения классов IoC, DI и интерфейсов, просто чтобы включить модульное тестирование с использованием инструментов проверки и модульного тестирования, которые требуют этих изменений кода. Я не думаю, что они полезны, не когда вы видите код, который был довольно простым и понятным, превращается в кошмарный беспорядок сложных взаимодействий, полностью обусловленный необходимостью сделать каждый метод класса полностью отделенным от всего остального. , И чтобы добавить оскорбление к травме, у нас есть много аргументов о том, должны ли частные методы тестироваться модульно или нет! (конечно, они должны, что

Проблема, конечно же, в природе испытательного инструментария.

В настоящее время доступны лучшие инструменты, которые могут навсегда оставить эти изменения в дизайне. В Microsoft есть Fakes (урожденные родинки), которые позволяют вам заглушать конкретные объекты, в том числе статические, так что вам больше не нужно менять код в соответствии с инструментом. В вашем случае, если вы использовали Fakes, вы бы заменили вызов GetTypes своим собственным, который возвращал действительные и недействительные тестовые данные - что очень важно, предлагаемое вами изменение вообще не предусматривает этого.

Ответить: ваш коллега прав, но, возможно, по неправильным причинам. Не меняйте код для тестирования, меняйте свой инструмент тестирования (или всю свою стратегию тестирования, чтобы иметь больше юнит-тестов в стиле интеграции вместо такого детального тестирования).

Мартин Фаулер обсуждает эту тему в своей статье.

gbjbaanb
источник
1

Хорошей и распространенной практикой является использование модульного тестирования и отладки журналов . Модульные тесты гарантируют, что если вы сделаете какие-либо дальнейшие изменения в программе, ваша старая функциональность не будет нарушена. Журналы отладки могут помочь вам отследить программу во время выполнения.
Иногда бывает так, что даже сверх этого нам нужно что-то только для целей тестирования. Нет ничего необычного в том, чтобы изменить код для этого. Но следует соблюдать осторожность, чтобы не повредить производственный код из-за этого. В C ++ и C это достигается с помощью MACRO , который является сущностью времени компиляции. Тогда тестовый код вообще не входит в картину в производственной среде. Не знаю, есть ли такое положение в C #.
Также, когда вы добавляете тестовый код в вашу программу, он должен быть четко виденчто эта часть кода добавлена ​​в целях тестирования. Или же разработчик, пытающийся понять код, просто потеет над этой частью кода.

Маной Р
источник
1

Есть некоторые серьезные различия между вашими примерами. В случае DoSomethingOnAllTypes(), есть значение, do somethingкоторое применимо к типам в текущей сборке. Но DoSomething(Assembly asm)явно указывает, что вы можете передать ему любую сборку.

Причина, по которой я указываю на это, заключается в том, что многие этапы внедрения зависимостей только для тестирования выходят за пределы исходного объекта. Я знаю, что вы сказали « не делать все общедоступным », но это одна из самых больших ошибок этой модели, за которой следует следующая: открытие методов объекта для целей, для которых они не предназначены.

Росс Паттерсон
источник
0

Ваш вопрос дал мало контекста, в котором ваш коллега спорил, так что есть место для спекуляций

«плохая практика» или нет, зависит от того, как и когда внесены изменения.

В моем варианте ваш пример для извлечения метода DoSomething(types)в порядке.

Но я видел код, который не так, как это:

public void DoSomethingOnAllTypes()
{
  var types = (!testmode) 
      ? Assembly.GetExecutingAssembly().GetTypes() 
      : getTypesFromTestgenerator();

  foreach (var currentType in types)
  {
     if (!testmode)
     {
        // do something with this type that made the unittest fail and should be skipped.
     }
     // do something with this type (e.g: read it's attributes, process, etc).
  }
}

Эти изменения сделали код более сложным для понимания, потому что вы увеличили количество возможных путей кода.

Что я имею в виду, как и когда :

если у вас есть работающая реализация и ради «реализации возможностей тестирования» вы внесли изменения, вам придется повторно протестировать ваше приложение, потому что вы, возможно, нарушили свой DoSomething()метод.

Это if (!testmode)сложнее понять и проверить, чем извлеченный метод.

k3b
источник