В программировании и в TDD, в частности, есть хорошие принципы, которых полезно придерживаться: DRY и тестирование через public-методы. Они неоднократно оправдали себя на практике, но в проектах с большим legacy-кодом могут иметь «тёмную сторону». Например, ты можешь писать код, руководствуясь этими принципами, а потом обнаружить себя разбирающим тесты, охватывающие связку из 20+ абстракций с конфигурацией, несоизмеримо превосходящей по объему тестируемую логику. Эта «тёмная сторона» пугает людей и тормозит использование TDD в проектах. Под катом я рассуждаю, почему тестировать через public-методы плохо и как можно уменьшить проблемы, возникающие из-за этого принципа.
Идея, лежащия в основе принципа, звучит хорошо: тестировать нужно поведение, а не реализацию. А значит, тестировать нужно только интерфейс класса. На практике это не всегда так. Чтобы представить суть проблемы представьте, что у вас есть метод, рассчитывающий стоимость работников занятых на сменной работе. Это нетривиальная задача, если речь идет о сменной работе, т.к. у них есть чаевые, бонусы, выходные, праздники, корпоративные правила и пр. и пр. Этот метод внутри выполняет очень много операций и использует другие сервисы, которые ему дают информацию о праздниках, чаевых и пр. Т.о. при написании для него юнит-теста необходимо создать конфигурацию для всех используемых сервисов, если проверяемый код находится где-то в конце метода. При этом, сам проверяемый код может использовать только частично, или вообще не использовать конфигурируемые сервисы. И есть уже некоторые юнит-тесты, написанные подобным образом.
Теперь вы хотите добавить реакцию на новый признак, который имеет нетривиальную логику и используется так же где-то в конце метода. Характер флага таков, что он является частью сервисной логики и, в тоже время, не является частью интерфейса сервиса. В приведенном случае, данный код актуален только для этого public-метода, и может быть, вообще, вписан внутрь старого метода.
Если в проекте принято правило тестировать всё только через public-методы, то разработчик может, скорее всего, просто скопирует какой-то существующий юнит-тест и немного его подправит. В новом тесте всё так же будет конфигурация всех сервисов для запуска метода. С одной стороны, принцип мы соблюли, но, с другой стороны, получили юнит-тест с оверконфигурацией. В дальнейшем, если что-то сломается, или потребует изменения конфигурации, придется делать мартышкину работу по корректировке тестов. Это нудно, долго и не приносит ни радости, ни видимой пользы клиенту. Казалось бы, следуем правильному принципу, но оказываемся в той же ситуации, от которой хотели уйти, отказываясь от тестирования private-методов.
Дальше может вмешаться такой человеческий фактор, как лень. Например, private-метод с нетривиальной логикой флага может выглядеть так, как в этом примере.
Данный метод требует 10 проверок, чтобы покрыть все случаи, 8 из них существенные.
При написании юнит-тестов для проверки этой логики, разработчику придется написать не меньше 8 больших юнит-тестов. Я сталкивался со случаями, когда конфигурация юнит-теста занимала 50+ строк кода, при 4 строках непосредственного вызова. Т.е. только, примерно, 10% кода несёт полезную нагрузку. В этом случае велик соблазн сократить объем работы за счет написания меньшего количества юнит-тестов. В итоге из 8 остается, например, только два юнит теста, для каждого значения clockIn. Такая ситуация приводит к тому, что либо, опять же, нудно и долго надо писать все необходимые тесты, создавая конфигурацию (Ctrl+C,V работает, куда же без него), либо метод остаётся только частично покрытым. У каждого варианта есть свои неприятные последствия.
Кроме принципа «тестироват поведение», еще есть OCP (Open/closed principle). Применяя его правильно, вы можете забыть что такое «хрупкие тесты», тестируя внутреннее поведение модуля. Если вам понадобится новое поведение модуля, вы напишете новые юнит-тесты для нового класса-наследника, в котором будет изменено нужное вам поведение. Тогда вам не надо будет тратить время на перепроверку и корректировку существующих тестов. В этом случае данный метод можно объявить, как internal, или protected internal, и тестировать, добавив InternalsVisibleTo к сборке. В этом случае ваш IClass интерфейс не пострадает, а тесты будут наиболее локаничными, не подверженными частым изменениям.
Другой альтернативой может быть объявление дополнительного класса-хелпера, в который можно вытащить наш метод, объявив его, как public. Тогда и принцип будет соблюдён, и тест будет лаконичным. На мой взгляд, такой подход не всегда себя оправдывает. Например, некоторые могут решить вытаскивать даже один метод в один класс, что приводит к созданию кучи классов с одним методом. Другой крайностью может быть сваливание таких методов в один класс-хелпер, который превращается в GOD-helper-class. Но этот вариант с хелпером может быть единственным, если рабочая сборка подписана строгим именем, а тестовую сборку вы подписать не можете, по каким-то причинам. InternalsVisibleTo будет работать когда сразу обе сборки либо подписаны, либо нет.
И в итоге, из-за совокупности подобных проблем страдает идея TDD и юнит-тестов, т.к. желания писать объемные тесты и поддерживать их нет ни у кого. Буду рад любым примерам того, как четкое следование данному принципу приводило к проблемам и снижению мотивации писать тесты у команды разработчиков.
Отказ от ответственности
Сразу хочу развеять возможное впечатление. Некоторые, возможно, даже не ощущают тех минусов, о которых пойдет речь, в силу, например, размера их проектов. Так же, данные недостатки, на мой взгляд, являются частью технического долга и обладают такими же характеристиками: проблема будет расти, если ей не уделять внимания. Поэтому решать необходимо по ситуации.
Идея, лежащия в основе принципа, звучит хорошо: тестировать нужно поведение, а не реализацию. А значит, тестировать нужно только интерфейс класса. На практике это не всегда так. Чтобы представить суть проблемы представьте, что у вас есть метод, рассчитывающий стоимость работников занятых на сменной работе. Это нетривиальная задача, если речь идет о сменной работе, т.к. у них есть чаевые, бонусы, выходные, праздники, корпоративные правила и пр. и пр. Этот метод внутри выполняет очень много операций и использует другие сервисы, которые ему дают информацию о праздниках, чаевых и пр. Т.о. при написании для него юнит-теста необходимо создать конфигурацию для всех используемых сервисов, если проверяемый код находится где-то в конце метода. При этом, сам проверяемый код может использовать только частично, или вообще не использовать конфигурируемые сервисы. И есть уже некоторые юнит-тесты, написанные подобным образом.
Минус 1: Оверконфигурация юнит-тестов
Теперь вы хотите добавить реакцию на новый признак, который имеет нетривиальную логику и используется так же где-то в конце метода. Характер флага таков, что он является частью сервисной логики и, в тоже время, не является частью интерфейса сервиса. В приведенном случае, данный код актуален только для этого public-метода, и может быть, вообще, вписан внутрь старого метода.
Если в проекте принято правило тестировать всё только через public-методы, то разработчик может, скорее всего, просто скопирует какой-то существующий юнит-тест и немного его подправит. В новом тесте всё так же будет конфигурация всех сервисов для запуска метода. С одной стороны, принцип мы соблюли, но, с другой стороны, получили юнит-тест с оверконфигурацией. В дальнейшем, если что-то сломается, или потребует изменения конфигурации, придется делать мартышкину работу по корректировке тестов. Это нудно, долго и не приносит ни радости, ни видимой пользы клиенту. Казалось бы, следуем правильному принципу, но оказываемся в той же ситуации, от которой хотели уйти, отказываясь от тестирования private-методов.
Минус 2: Неполное покрытие
Дальше может вмешаться такой человеческий фактор, как лень. Например, private-метод с нетривиальной логикой флага может выглядеть так, как в этом примере.
private bool HasShifts(DateTime date, int tolerance, bool clockIn, Shift[] shifts, int[] locationIds)
{
bool isInLimit(DateTime date1, DateTime date2, int limit)
=> Math.Abs(date2.Subtract(date1).TotalMinutes) <= limit;
var shiftsOfLocations = shifts.Where(x => locationIds.Contains(x.LocationId));
return clockIn
? shiftsOfLocations.Any(x => isInLimit(date, x.StartDate, tolerance))
: shiftsOfLocations.Any(x => isInLimit(date, x.EndDate, tolerance));
}
Данный метод требует 10 проверок, чтобы покрыть все случаи, 8 из них существенные.
Расшифровка 8 важных кейзов
Итого: 2 x 2 x 2 = 8
- shiftsOfLocations — 2 значения — есть или нет
- clockIn — 2 значения — true или false
- tolerance — 2 разных значения
Итого: 2 x 2 x 2 = 8
При написании юнит-тестов для проверки этой логики, разработчику придется написать не меньше 8 больших юнит-тестов. Я сталкивался со случаями, когда конфигурация юнит-теста занимала 50+ строк кода, при 4 строках непосредственного вызова. Т.е. только, примерно, 10% кода несёт полезную нагрузку. В этом случае велик соблазн сократить объем работы за счет написания меньшего количества юнит-тестов. В итоге из 8 остается, например, только два юнит теста, для каждого значения clockIn. Такая ситуация приводит к тому, что либо, опять же, нудно и долго надо писать все необходимые тесты, создавая конфигурацию (Ctrl+C,V работает, куда же без него), либо метод остаётся только частично покрытым. У каждого варианта есть свои неприятные последствия.
Возможные решения
Кроме принципа «тестироват поведение», еще есть OCP (Open/closed principle). Применяя его правильно, вы можете забыть что такое «хрупкие тесты», тестируя внутреннее поведение модуля. Если вам понадобится новое поведение модуля, вы напишете новые юнит-тесты для нового класса-наследника, в котором будет изменено нужное вам поведение. Тогда вам не надо будет тратить время на перепроверку и корректировку существующих тестов. В этом случае данный метод можно объявить, как internal, или protected internal, и тестировать, добавив InternalsVisibleTo к сборке. В этом случае ваш IClass интерфейс не пострадает, а тесты будут наиболее локаничными, не подверженными частым изменениям.
Другой альтернативой может быть объявление дополнительного класса-хелпера, в который можно вытащить наш метод, объявив его, как public. Тогда и принцип будет соблюдён, и тест будет лаконичным. На мой взгляд, такой подход не всегда себя оправдывает. Например, некоторые могут решить вытаскивать даже один метод в один класс, что приводит к созданию кучи классов с одним методом. Другой крайностью может быть сваливание таких методов в один класс-хелпер, который превращается в GOD-helper-class. Но этот вариант с хелпером может быть единственным, если рабочая сборка подписана строгим именем, а тестовую сборку вы подписать не можете, по каким-то причинам. InternalsVisibleTo будет работать когда сразу обе сборки либо подписаны, либо нет.
Итог
И в итоге, из-за совокупности подобных проблем страдает идея TDD и юнит-тестов, т.к. желания писать объемные тесты и поддерживать их нет ни у кого. Буду рад любым примерам того, как четкое следование данному принципу приводило к проблемам и снижению мотивации писать тесты у команды разработчиков.
