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

    Разработчик обязан знать свои инструменты! Знание инструментов увеличивает продуктивность, эффективность, производительность, потенцию разработчика! Не могу программировать без 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% покрытия — вина может поделиться между руководством и разработчиками. Вопрос линии раздела вины поднимать не стоит, ибо холивар.

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

    Похожие публикации

    Комментарии 32

      +5
      Проблема №1 прекрасно иллюстрирует, что любая планка ниже 100% приводит к тому, что покрывается то что легче, а не то что нужно.

      Есть ещё и такое мнение.
        +2
        Помечать код как «Не анализировать покрытие» — подход заслуживающий права на жизнь. К сожалению, им тоже можно злоупотребить. Доводя до абсурда — можно пометить весь код «неанализируемым».
        Но статья отличная, спасибо за линк.
          +2
          Пометка будет иметь объяснение и коммитера.
            0
            Несколько недостатков по-прежнему остается.
            Например, 'commited by Vasya, «Will test later, deadline»'. Или проекты где анализ внедряется после 5 лет разработки: в наличии миллион строк кода, покрытие — 50%. Метить непокрытый код проблемно, придется реагировать на изменения.

            Но в целом вариант очень даже интересный, с явными преимуществами перед 80% покрытия.
        +1
        В последнем предложении взаимоисключающие параграфы, не?
        Если не гнаться за цифрами, то это не метрика.
          0
          Спасибо, поправил.
          +1
          Интересно, существуют ли тулзы для вычисления code coverage, которые могут игнорировать не-покрытие фрагмента кода, если рядом с ним есть сигнальный комментарий. Казалось бы, в таком случае правило «весь код должен быть покрыт на 100% либо иметь разумную причину быть не покрытым, выраженную в комментарии» решает большую часть проблем, указанных в статье.
            0
            Думаю, зависит от языка\инструмента. По ссылке от andy128k (см. первый комментарий) можно найти вот этот пример.
            +2

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


            То есть правильная характеристика — не покрытие кода, а скорее "мера непокрытого кода".


            Ну и конечно — адекватность тестов должна проверяться отдельно на code review, тут уже никто не спорит. Накрутки тестов и покрытие должны исключаться и удаляться.

              0
              Еще лучше: проверять адекватность тестов\CC еще в процессе разработки. Во время CodeReview проблемно проверить какие именно участки код покрыты.

              Code Review позволяет прикинуть какие сценарии коммитер протестировал, но только прикинуть. CC инструмент же заодно подсвечивает те строки метода, которые не покрыты.

              Несомненно, предварительный просмотр кода CC утилитой не отменяет CodeReview, однако и CodeReview предварительного просмотра полностью не заменит.
              +1

              Не забывайте что есть другие подходы, например Property Based Testing или Mutation Testing, которые друг друга дополняют и могут заменить классический ААА путь написания тестов. Но защиты от сумасшедшего индуса дурка нет.

                0
                Любой процент ппокрытия не говорит вообще ничего, потому что эта метрика никак не связана с качеством тестов.
                Пример:
                if(a > 5){

                } else {

                }
                Вы пишете тест где а = 10 и а = 3. Анализ покрытия скажет вам, что код протестен на 100%. Но говорит ли этот факт о том, что условие действительно корректно протестено? Конечно нет, ибо если вдруг поменяем условие на а >=5 тесты все еще будут зелеными, хотя поведение программы изменилось.

                Поэтому можно говорить только о том, что покрытие в 70% наверное лутше чем в 50% и не более того.

                Что бы улутшить ситуацию, надо тестить сами тесты. Например через mutation testing. На хабре есть статьи на эту тему, очень рекомендую.
                  +1
                  Анализ покрытия скажет вам, что код протестен на 100%

                  Это необразованный человек так интерпретирует результаты анализатора. Анализ покрытия говорит, что все ветки кода исполнялись хотя бы раз. Ничего более.

                    +1
                    Вы пишете тест где а = 10 и а = 3.

                    Наш код стоит тестировать примерами a=5 и a=4, как два граничных случая. Соответственно, появится красный тест при изменении условия на a >= 5.

                    Любой процент ппокрытия не говорит вообще ничего, потому что эта метрика никак не связана с качеством тестов.

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

                    Что до мутационного тестирования — это (интересная) тема отдельных статей на хабре.

                      +1
                      Мутацинное тестирование (и/или фаззинг) это приятное дополнение к покрытию, а не замена.
                      +5
                      Я бы посоветовал относиться к покрытию кода как температуре тела — 36.6 не говорит, что человек здоров, но большие отклонения от нормы обычно говорят о серьезных проблемах.
                      Нужно ли мерить покрытие кода авто-тестами? Мое мнение — обязательно. И в первую очередь не ради магической цифры, а для анализа результатов.
                      Да, как и любую другую метрику ее можно скомпрометировать.
                      Да, метрика является линейной — не может отличить полезный код от мало- или бесполезного. Но если фокусироваться на анализе, то смотреть можно на покрытие отдельных компонент или библиотек. При желании можно как-то разметить код и получить нелинейную метрику (примерно как раскладка багов по северити).
                      Да, метрика не показывает отсутствующие ветки кода или покрытие граничных условий. Примите как ограничение. Покрытие требований избавлено от этих недостатков? Вы все граничные условия прописываете в требования?

                      Я бы выделил следующие сильные стороны измерения и анализа покрытия кода по сравнению с покрытием требований:
                      1. тестовое покрытие можно мерить при отсутствии требований или других артефактов.
                      2. метрика инструментальная, т.е. ее получение обычно относительно недорого (в плане трудозатрат).
                      3. Т.ч. процесс измерений автоматизирован, то исключен человеческий фактор — труднее манипулировать результатом.
                      4. Т.к. процесс относительно дешев, то можно мерить часто и отслеживать тренды.

                      И если смотреть на тренды, а не на абсолютные цифры, то легко ответить на данные вопрос " Нужно ли покрывать мертый код?" — Нет, не нужно. Если он реально мертвый, то нужно выбросить. Если он редкоиспользуемый и мы не хотим его покрывать, но тренд все равно будет растущий (при условии, что новый код покрыт на достаточном уровне).

                      И опять же, любую полезную метрику можно превратить во вредный KPI, т.ч. я бы не стал на этом моменте фокусироваться. Гораздо интереснее сопоставлять сильные и слабые стороны разных метрик оценки тестового покрытия.
                        0

                        СС — это технологическая метрика. Она не много говорит о качестве программного продукта. Зато кое-что говорит о качестве процесса его разработки!

                          +1
                          А можно развернуть мысль?
                          Спасибо!
                            0

                            Пример:
                            Предположим, на следующий год вы можете влиться в одну из двух команд.
                            Всё примерно одинаково: по 4 человека в командах, тип приложения в обоих случаях "как вы любите", размеры исходного кода, количество пользователей, выручка… всё примерно одинаково.
                            Но в одном случае у проекта на дашборде светится 62.4% code coverage (а на начало прошлого года — 55%), а у другого проекта этой цифры вообще нет и никто даже примерно ее не знает.
                            В какую команду вы пойдете?

                              +1
                              В какую команду вы пойдете?

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

                                +1
                                Я бы пошел во вторую — выстраивать процессы.
                                А разработчикам посоветовал бы идти в первую. Т.к. по моим внутренним убеждениям, при прочих равных она выиграет в долгосрочной перспективе. Технический долг сказывается на дистанции, я бы предположил, что во второй команде он выше.
                            0

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

                              0
                              (поскольку создана для Васи, который будет её юзать через два дня).

                              А почему юнит тест не может это покрыть?

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

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

                                  Если это большой метод, то почему нельзя его вынести в отдельный класс или написать test-specific descendant для тестирования или сделать публичным, если это маленький метод почему нельзя чтобы коллега его добавил при разработке.

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

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

                                    В итоге мы получим некоторое количество геморроя лишь для «100% покрытия».

                                    Возвращаясь к оригинальной фразе (часть кода… недостижима (поскольку создана для Васи, который будет её юзать через два дня).) — вы правы, с недостижимостью я косякнул.
                                      0

                                      Фактически вы разрабатваете что-то что используется Васей как отдельный модуль. То есть есть high cohension внутри этого метода и low coupling с Васиным кодом. Сам по себе факт того, что это надо это признак для рефакторинга. Тесты нужны и для того, чтобы выявлять такие моменты.

                                0
                                Покрытие тестами — это метрика, того как на фирме относятся к качеству кода.

                                Наличие на предприятии директора по качеству, ОТК и сертификатов качества, еще не говорит, что продукт качественный(директор — разгильдяй, сертификаты липовые), но по крайней мере на этом заводе знают слово «качество»)))
                                  +2

                                  Для меня тесты — это не только показатель работоспособности кода, но и инструмент для написания более архитектурно грамотного, качественного кода. Следуя TDD проблемные места видны заранее, еще до того как был реализован класс.
                                  По этой причине низкое покрытие тестами может быть сигналом о том, что присутствуют архитектурные проблемы. На практике я заметил, что практически всегда тот код, который нельзя/сложно протестировать — код низкого качества.
                                  Поэтому я стараюсь достигать как можно больших значений CC, пытаясь не "читерить".

                                    +1
                                    SonarQube, а не SonarCube
                                      0
                                      ниодин code coverage tool еще не смог показать честное покрытие. Особенно, в Javascript, где особенности языка позволяют еще большие трюки, чем в Java. Поэтому эти метрики носят лишь рекомендательный сферический характер. Чтоб покрыть функционал ( а не строки кода) на 100%, нужны хорошие тесты. Разработчики часто не умеют или не хотят тратить на это время. Тесты становятся болью, теряется смысл их написания и поддержки.
                                        +1
                                        ниодин code coverage tool еще не смог показать честное покрытие
                                        Странно, у меня совсем другой опыт: любой code coverage tool показывает честное покрытие до тех пор, пока вы не пытаетесь его обойти. Может, от языка зависит.

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

                                      Самое читаемое