Правила внедрения TDD в старом проекте

Статья «Скользящая ответственность паттерна Репозиторий» подняла несколько вопросов, на которые очень сложно дать ответ. Нужен ли репозиторий, если абстрагироваться от технических деталей полностью невозможно? На сколько сложным репозиторий может быть, чтобы его написание оставалось целесообразным? Ответ на эти вопросы различается в зависимости от акцента, который делается при разработке систем. Наверно, самый сложный вопрос: нужен ли, вообще, репозиторий? Проблема «текучей абстракции» и рост сложности кодирования с увеличением уровня абстракции не позволяют найти решение, которое удовлетворяло бы оба лагеря. Например, в репортинге intention design приводит к созданию большого числа методов для каждого фильтра и сортировки, а generic решение создает большой оверхед по кодированию. Продолжать можно бесконечно…

Для более полного представления я взглянул на проблему абстракций со стороны применения их в уже готовом коде, в legacy code. Репозиторий, в таком случае, нас интересует только, как инструмент для достижения качественного и безбажного кода. Конечно, этот паттерн — не единственное, что необходимо для применения TDD практик. Наевшись «невкусной еды» в нескольких больших проектах и наблюдая за тем, что работает, а что нет, я вывел для себя несколько правил, которые мне помогают следовать TDD практикам. С удовольствием выслушаю конструтктивную критику и иные приёмы внедрения TDD.

Предисловие

Некоторые могут заметить, что в старом проекте применить TDD невозможно. Существует мнение, что для них больше подходят разные виды интеграционных тестов (UI-тесты, end-to-end), т.к. разобраться в старом коде слишком сложно. Так же, можно услышать, что написание тестов перед самим кодированием приводит только к потере времени, т.к. мы можем не знать, как будет работать код. Мне приходилось работать в нескольких проектах, где ограничивались только интеграционными тестами, считая, что юнит-тесты не показательны. При этом писалось очень много тестов, они запускали кучу сервисов и пр. и пр. В итоге разобраться в этом мог только один человек, который их, собственно, и написал.

За свою практику я успел поработать в нескольких очень крупных проектах, где было очень много legacy code. В одних были тесты, в других только собирались это внедрять. Мне самому удалось поднять 2 больших проекта. И везде я так или иначе пытался применять TDD подход. На начальных этапах понимания TDD воспринимался, как Test First development. Но чем дальше, тем отчетливее были видны отличия между этим упрощенным пониманием и нынешнем представлением, называемым коротко BDD. Какой бы язык не использовался, основные моменты, которые я назвал правилами, остаются похожими. Кто-то может найти параллели между правилами и другими принципами написания хорошего кода.

Правило 1: Используем Bottom-Up (Inside-Out)

Это правило больше относится к способу анализа и дизайна ПО при встраивании новых кусков кода в уже работающий проект.

Когда вы проектируете новый проект, абсолютно естественно представлять систему целиком. На этом этапе вы контролируете и набор компонентов, и будущую гибкость архитектуры. Поэтому можете писать модули, которые удобным и лучшим образом интегрируются друг с другом. Такой Top-Down подход позволяет выполнить хороший upfront дизайн будущей архитектуры, описать необходимые гуайдлайны и иметь целостное представление того, что, в итоге, хочется. Через некоторое время проект превращается в то, что называют legacy code. И тут начинается самое интересное.

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

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

Правило 2: Тестируйте только изменённый код

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

Пример

Существует модуль online-магазина, который создает корзину из выбранных элементов и сохраняет её в базу. Конкретная реализация нас не волнует. Как сделано, так сделано — это legacy code. Теперь нам необходимо внедрить сюда новое поведение: отправлять уведомление в бухгалтерию в случае, когда стоимость корзины превышает 1000$. Вот такой код мы видим. Как внедрить изменение?

public class EuropeShop : Shop
{
    public override void CreateSale()
    {
        var items = LoadSelectedItemsFromDb();
        var taxes = new EuropeTaxes();
        var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList();
        var cart = new Cart();
        cart.Add(saleItems);
        taxes.ApplyTaxes(cart);
        SaveToDb(cart);
    }
}

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

public class EuropeShop : Shop
{
    public override void CreateSale()
    {
        var items = LoadSelectedItemsFromDb();
        var taxes = new EuropeTaxes();
        var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList();
        var cart = new Cart();
        cart.Add(saleItems);
        taxes.ApplyTaxes(cart);

        // NEW FEATURE
        new EuropeShopNotifier().Send(cart);

        SaveToDb(cart);
    }
}

Такой нотификатор является рабочим сам по себе, может быть протестирован, а внесенные изменения в старый код минимальны. Это именно то, о чём гласит второе правило.

Правило 3: Тестируем только требования

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

Так же представьте, как другие классы, являющиеся клиентами вашего модуля, будут с ним взаимодействовать. Нужно ли им писать 10 строк кода, чтобы настроить ваш модуль? Чем проще будут коммуникации между частями системы, тем лучше. Поэтому из старого кода лучше выделять модули, ответственные за что-то конкретное. Тут вам на помощь придёт SOLID.

Пример

Теперь посмотрим, как всё о чем писал выше нам поможет с кодом. Сначала выделим все модули, которые только косвенно связаны с созданием корзины. Вот так распределена ответственность по модулям.

public class EuropeShop : Shop
{
    public override void CreateSale()
    {
        // 1) load from DB
        var items = LoadSelectedItemsFromDb();

        // 2) Tax-object creates SaleItem and
        // 4) goes through items and apply taxes
        var taxes = new EuropeTaxes();
        var saleItems = items.Select(item => taxes.ApplyTaxes(item)).ToList();

        // 3) creates a cart and 4) applies taxes
        var cart = new Cart();
        cart.Add(saleItems);
        taxes.ApplyTaxes(cart);

        new EuropeShopNotifier().Send(cart);

        // 4) store to DB
        SaveToDb(cart);
    }
}

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

public class EuropeShop : Shop
{
    public override void CreateSale()
    {
        // 1) extracted to a repository
        var itemsRepository = new ItemsRepository();
        var items = itemsRepository.LoadSelectedItems();
			
        // 2) extracted to a mapper
        var saleItems = items.ConvertToSaleItems();
			
        // 3) still creates a cart
        var cart = new Cart();
        cart.Add(saleItems);
			
        // 4) all routines to apply taxes are extracted to the Tax-object
        new EuropeTaxes().ApplyTaxes(cart);
			
        new EuropeShopNotifier().Send(cart);
			
        // 5) extracted to a repository
        itemsRepository.Save(cart);
    }
}

Что касается тестов, то можно обойтись данными сценариями. Пока их реализация нас не интересует.

public class EuropeTaxesTests
{
    public void Should_not_fail_for_null() { }

    public void Should_apply_taxes_to_items() { }

    public void Should_apply_taxes_to_whole_cart() { }

    public void Should_apply_taxes_to_whole_cart_and_change_items() { }
}

public class EuropeShopNotifierTests
{
    public void Should_not_send_when_less_or_equals_to_1000() { }

    public void Should_send_when_greater_than_1000() { }

    public void Should_raise_exception_when_cannot_send() { }
}

Правило 4: Добавляем только протестированный код

Как я уже писал выше, следует минимизировать изменения в старый код. Чтобы это сделать, старый и новый/модифицированный код можно разделить. Новый код можно выделить в методы, работу которых можно проверить юнит тестами. Такой подход поможет уменьшить связанные риски. Существует две техники, которые были описаны в книге «Working Effectively with Legacy Code» (ссылка на книгу ниже).

Sprout method/class — эта техника позволяет встроить очень безопасно новый код в старый. То, как я добавил нотификатор и является примером данного подхода.

Wrap method — несколько посложнее, но суть такая же. Подходит не всегда, а только в случаях когда новый код вызывается до/после старого. При выделении ответственностей два вызова метода ApplyTaxes заменились одним вызовом. Для этого надо было поменять второй метод так, чтобы логика работы не сломалась сильно и её можно было проверить. Вот так выглядел класс до изменений.

public class EuropeTaxes : Taxes
{
    internal override SaleItem ApplyTaxes(Item item)
    {
        var saleItem = new SaleItem(item)
        {
            SalePrice = item.Price*1.2m
        };
        return saleItem;
    }

    internal override void ApplyTaxes(Cart cart)
    {
        if (cart.TotalSalePrice <= 300m) return;
        var exclusion = 30m/cart.SaleItems.Count;
        foreach (var item in cart.SaleItems)
            if (item.SalePrice - exclusion > 100m)
                item.SalePrice -= exclusion;
    }
}

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

public class EuropeTaxes : Taxes
{
    internal override void ApplyTaxes(Cart cart)
    {
        ApplyToItems(cart);
        ApplyToCart(cart);
    }

    private void ApplyToItems(Cart cart)
    {
        foreach (var item in cart.SaleItems)
            item.SalePrice = item.Price*1.2m;
    }

    private void ApplyToCart(Cart cart)
    {
        if (cart.TotalSalePrice <= 300m) return;
        var exclusion = 30m / cart.SaleItems.Count;
        foreach (var item in cart.SaleItems)
            if (item.SalePrice - exclusion > 100m)
                item.SalePrice -= exclusion;
    }
}

Правило 5: «Ломаем» скрытые зависимости

Это правило о самом большом зле в старом коде: об использовании оператора new внутри метода одного BO для создания других BO, репозиториев или других непростых объектов. Почему это плохо? Самое простое объяснение: это делает части системы сильно связанными и способствует уменьшению их согласованности. Еще короче: приводит к нарушению принципа «low coupling, high cohesion». Если взглянуть с другой стороны, то такой код слишком сложно будет выделить в отдельный, независимый инструмент. Избавиться от таких скрытых зависимостей за раз очень трудоёмко. Но это можно делать постепенно.

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

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

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

Пример

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

public class EuropeShop : Shop
{
    private readonly IItemsRepository _itemsRepository;
    private readonly Taxes.Taxes _europeTaxes;
    private readonly INotifier _europeShopNotifier;

    public EuropeShop()
    {
        _itemsRepository = new ItemsRepository();
        _europeTaxes = new EuropeTaxes();
        _europeShopNotifier = new EuropeShopNotifier();
    }

    public override void CreateSale()
    {
        var items = _itemsRepository.LoadSelectedItems();
        var saleItems = items.ConvertToSaleItems();

        var cart = new Cart();
        cart.Add(saleItems);

        _europeTaxes.ApplyTaxes(cart);
        _europeShopNotifier.Send(cart);
        _itemsRepository.Save(cart);
    }
}

Правило 6: Чем меньше больших тестов, тем лучше

Большие тесты — это различные интеграционные тесты, которыми пытаются тестировать пользовательские сценарии. Несомненно, они важные, но проверять ими логику некоторого IF в глубине кода очень затратно. В итоге только один девелопер, только в специальном костюме, обложившись оберегами, сможет там что-то менять. Написание такого теста требует столько же времени, если не больше, как и написание самого функционала. Поддержка их — это как еще один legacy code, который страшно менять. При этом это всего лишь тесты!

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

Правило 7: Не тестируем private методы

Если вдруг вам захотелось протестировать приватный метод, то, видимо, вы истосковались по костылям. Некоторые не видят в этом ничего плохого. Но давайте посмотрим на причины вашей «хотелки». Приватный метод может быть слишком сложным или содержать код, который не вызывается из публичных методов. Уверен, что любая другая причина, которую можно придумать, окажется характеристикой «плохого» кода или дизайна. Скорее всего, часть кода из приватного метода должна выделиться в отдельный метод/класс. Проверьте не нарушается ли первый принцип SOLID? Эта первая причина почему так не стоит делать. Вторая — это то, что таким образом вы проверяете не поведение всего модуля, а то, как он это делает. Внутренняя реализация может меняться независимо от поведения модуля. Поэтому в таком случае вы получаете хрупкие тесты, на поддержку которые тратится больше времени, чем необходимо.

Чтобы избежать необходимости тестировать приватные методы, представте свои классы, как набор атомарных инструментов, об устройстве которых вы не знаете ничего. Вы ожидаете некоторое поведение, которое и тестируете. Этот взгляд справедлив и для классов в рамках ассембли. Классы, доступные клиентам (из других ассембли) будут паблик, а те, что выполняют внутреннюю работу private. Хотя, отличие от методов есть. Классы внутреннего назначения могут быть сложными, поэтому их можно сделать internal и тоже тестировать.

Пример

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

public class EuropeTaxes : Taxes
{
    // code skipped

    private void ApplyToCart(Cart cart)
    {
        if (cart.TotalSalePrice <= 300m) return; // <<< I WANT TO TEST THIS CONDIFTION
        var exclusion = 30m / cart.SaleItems.Count;
        foreach (var item in cart.SaleItems)
            if (item.SalePrice - exclusion > 100m)
                item.SalePrice -= exclusion;
    }
}

// test suite
public class EuropeTaxesTests
{
    // code skipped

    [Fact]
    public void Should_apply_taxes_to_cart_greater_300()
    {
        #region arrange
        // list of items which will create a cart greater 300
        var saleItems = new List<Item>(new[]{new Item {Price = 83.34m},
            new Item {Price = 83.34m},new Item {Price = 83.34m}})
            .ConvertToSaleItems();
        var cart = new Cart();
        cart.Add(saleItems);

        const decimal expected = 83.34m*3*1.2m;
        #endregion

        // act
        new EuropeTaxes().ApplyTaxes(cart);

        // assert
        Assert.Equal(expected, cart.TotalSalePrice);
    }
}

Правило 8: Не тестируем алгоритм методов

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

Правило 9: Не модифицируем legacy code без тестов

Это самое главное правило, т.к. отражает желание команды следовать такому пути. Без желания двигаться в этом направлении всё, о чем было сказано выше, смысла особого не имеет. Т.к. если девелопер не хочет применять TDD (не понимает его смысл, не видит пользы и пр.), то реальная его польза будет размываться постоянным обсуждением как это тяжело и неэффективно.

Если вы собрались применять TDD, обсудите в команде, добавьте в Definition of Done и применяйте. Сначала будет тяжело, как со всем новым. Как и любое искусство, TDD требует постоянной практики, а удовольствие приходит по мере обучения. Постепенно, написанных юнит тестов станет много, вы начнете чувствовать здоровье вашей системы и начнете ценить простоту написания кода, описывая на первом этапе требования. Есть исследования TDD, проведенные на реальных больших проектах в Microsoft и IBM, показывающие уменьшение багов в продакшн системах от 40% до 80%. (см. ссылку ниже).

Дополнительно

1. Book “Working Effectively with Legacy Code” by Michael Feathers
2. TDD when up to your neck in Legacy Code
3. Breaking Hidden Dependencies
4. The Legacy Code Lifecycle
5. Should you unit test private methods on a class?
6. Unit testing internals
7. 5 Common Misconceptions About TDD & Unit Tests
8. Intro to Unit Testing 5: Invading Legacy Code in the Name of Testability
9. Law of Demeter