Pull to refresh

Code Coverage — хочу верить

Reading time4 min
Views40K
Разработчик обязан знать свои инструменты! Знание инструментов увеличивает продуктивность, эффективность, производительность, потенцию разработчика! Не могу программировать без R#!

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

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

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

«Есть ложь, есть наглая ложь
есть статистика».

Создатели SonarQube это великолепно понимают, недаром у SonarQube с десяток CC метрик. Я буду перебивать статистику используя CC от DevExpress, там метрика лишь одна.

Проблема 1. Типичный тест не того. Протестируем метод с кучей проверок аргумента:

public static Hash Example1_IfThen(int arg)
        {
            if(arg == 0) { throw new ArgumentException("0 argument"); }
            if (arg == 1) { throw new ArgumentException("1 argument"); }
            if (arg == 2) { throw new ArgumentException("2 argument"); }

            return new Hash(new Hash(arg + 42) + 13);
        }

... 

        [TestMethod, ExpectedException(typeof(ArgumentException))]
        public void Example1_IfThen_0()
            { Program.Example1_IfThen(0); }

        [TestMethod, ExpectedException(typeof(ArgumentException))]
        public void Example1_IfThen_2()
            { Program.Example1_IfThen(2); }

        [TestMethod, ExpectedException(typeof(ArgumentException))]
        public void Example1_IfThen_1()
            { Program.Example1_IfThen(1); }

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

Проблема 2. Замеряем актуальный код, вместо необходимого.

    public static int Example2_IncompleteLogic(IEnumerable<int> arg)
    {
        return arg
            .Where(elem => elem != 2)
            .Where(elem => elem != 3)
            .Count();
    }

    ...

    [TestMethod]
    public void OneTestToCoverAll()
    {
        // Arrange - collection with one element '4'
        var arg = new List<int> { 4 };

        // Act
        var result = Program.Example2_IncompleteLogic(arg);

        // Assert
        Assert.AreEqual(1, result);
    }

Тестируемый метод не содержит проверки на null аргумент, однако покрытие — 100%. Иногда люди забывают: Code Coverage — это метрика покрытия кода, не метрика закрытия требований; если в методе не достает логики (метод недостаточно сложен для решения своей задачи) — CC это не покажет.

100% покрытия не гарантируют работоспособности программы. Доводя до абсурда: пустой метод элементарно покрывается на 100%. Непустой метод покрывается на 100% тестами без Assert-ов.

Проблема 3. Оптимизм. Немного иное проявление предыдущей проблемы. Как видно, один тест покрывает 100% кода. Попробуем переписать наш метод, избавившись от LINQ (для улучшения производительности).

var result = 0;
foreach(var elem in arg)
{
    if(elem == 2 || elem == 3) { continue; }
    else { result++; }
}
return result;

Получаем лишь 73% покрытия. Функциональность не изменилась, метрика упала. Мало того, что 100% покрытия не гарантируют работоспособности программы, эти 100% могут быть фейковыми. Вывод: LINQ — г**но результаты CC могут быть завышены, старайтесь проверять покрытие в редакторе.

Побочное наблюдение: в данном случае мы можем просто иметь косяк в технической реализации, в теории анонимный метод elem => elem != 2 можно заменить на elem => if (elem != 2) return true else return false;, что пофиксит покрытие оригинального метода до 73%. Правда, такой подход потребует усложнения удобного сейчас UI.

Следствие: Используемый инструмент может не обладать всей желаемой функциональностью. Тривиальная вещь, не менее от того верная.

Проблема 4. Передача ответственности.

public static void MakeEverythingWell()
{
    OuterLib.MakeEverythingWell();
} 

Метод покрывается на 100% одним тестом. При этом покрытие OuterLib библиотеки лежит на совести того, кто её добавил. Или обновил. Года три назад, до введения CC. До увольнения.
Приходится снова констатировать факт: мало того, что 100% покрытия не гарантируют работоспособности программы, эти 100% могут быть фейковыми.

Помимо чисто кодовых моментов есть несколько претензий именно к обработке результатов CC


Претензия 0, всем известная. 100% покрытия. Нет 100% покрытия — нет одобрения билда. Проблема в том, что первые проценты покрытия получить относительно просто, а вот последние… Особенно, когда часть кода генерируется. Или недостижима (поскольку создана для Васи, который будет её юзать через два дня). Или просто теоретически достижима, а пример подбирать\высчитывать пару недель (такое бывает при работе с математикой). Короче, большинство команд (из тех кто вообще интегрирует CC в CI) останавливаются на 60\70\80 процентах необходимого покрытия.

Претензия 1, спорная. Покрытие мертвого кода. На моей памяти схожая проблема особо ярко проявилась в ходе проверки Mirand-ы коллегами из PVS. Комментарии довольно эмоциональны, но часть споров касалась мертвого кода: часть найденных диагностик указывала на (заброшенные) плагины, но не на ядро.

Возникает вопрос: нужен ли CodeCoverage для мертвого кода? С одной стороны, мертвый код это проблема, и привлечение внимания к нему приветствуется. С другой стороны, мертвый код не влияет на продакшн, так стоит ли позволять ему влиять на CC метрику?

Претензия 2. Важность кода. Расширение проблемы 1. В моем проекте есть два примечательных контроллера: «оплата» и «переговорка». «Оплата» критична для клиента, и я вполне согласен с требованием «80% покрытия», «переговоркой» же пользуются 1.5 анонимуса. В год. И она не менялась уже два года. Вопрос: для чего писать тесты к полумертвой функциональности? Лишь для получения 80% бейджа одобрения автосборки?

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

Претензия 4. Метрика «за бесплатно». Когда руководство скидывает требование «покрывайте код на 80%», и разработчики безропотно соглашаются. Проект при этом — одноразовый. Или прототип. Или дедлайн на носу. Или имеется здоровенный макаронный легаси монстр без единого теста.

Покрытие кода тестами требует времени! Если покрытие еще и замерять — время на тесты может и возрасти (хотя может и упасть). Так что если команда не успела сдать проект в срок, но зато достигла 80% покрытия — вина может поделиться между руководством и разработчиками. Вопрос линии раздела вины поднимать не стоит, ибо холивар.

Под конец. Еще раз замечу: СС — метрика полезная, хоть и с сюрпризами. Она реально помогает с контролем кода, если нет слепого стремления к цифрам в отчетах.
Tags:
Hubs:
Total votes 36: ↑35 and ↓1+34
Comments32

Articles