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

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

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

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

Просто простой пример, подумайте об этом фрагменте кода, который используется некоторым компонентом (написанным на 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 для тестирования. Мой коллега не думал, что это хорошая практика.

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

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

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

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

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

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

static void Main(string[] args)

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

В моем варианте ваш пример для извлечения метода 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)сложнее понять и проверить, чем извлеченный метод.