Привет, Хабр.

Меня зовут Андрей Белобров. Я тимлид одной из команд, разрабатывающих приложения для умных девайсов Сбера.

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

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

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

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

Стандарт кодирования

В любом относительно большом проекте есть свой стандарт кодирования, который обычно создается на базе одного или нескольких открытых стандартов. В нашем случае за основу был взят Google C++ Style Guide. Но, в отличие от стандарта Google, наш стандарт содержит ряд существенных отличий. Например, использование исключений у нас рекомендуется, а не запрещается.

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

Сейчас для максимальной переносимости кода мы используем C++ 17 и не используем возможности из более новых стандартов, даже если они поддерживаются в текущих версиях компиляторов.

Основные принципы

• Код стараемся писать максимально просто, так чтоб было понятно всем.

• Придя в новый код, не нужно все рефакторить.

• Преждевременная оптимизация — корень всех зол.

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

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

Автоматизация проверки стандарта кодирования

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

Поэтому требования нашего стандарта кодирования по форматированию кода успешно проверяются с помощью Clang-Format. Это позволяет тратить время непосредственно на ревью кода, без временных затрат на проверку форматирования.

Помимо форматирования некоторые пункты стандарта кодирования проверяются статическим анализом c использованием Clang-Tidy, например:

• Перегруженные методы помечаем словом override без virtual - modernize-use-override;

• Конструкторы с одним аргументом помечаем ключевым словом explicit - google-explicit-constructor;

• Не используем auto если выведенный тип не очевиден. В остальных случаях использование auto рекомендуется, так как делает код компактнее и проще для чтения - modernize-use-auto;

• Не используем magic numbers в коде, нужно выносить такие в константы с понятным названием - readability-magic-numbers.

Проверка readability-magic-numbers сейчас выключена, так как требует внести слишком много изменений, но мы считаем ее одной из самых полезных в Clang-Tidy и планируем в дальнейшем ее включить.

Ревью кода

Код-ревью является обязательным для каждого Pull Request-a перед тем как он попадет в основную ветку разработки.

Чтобы пройти ревью нужно получить:

• Минимум 2 подтверждения;

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

Компоненты представлены метками Bitbucket, в которых с помощью регулярных выражений заданы соответсвующие директории в исходном коде проекта.

Кроме непосредственно ревью кода, перед мержем Pull Request-а, нужно успешно пройти все автоматические сборки:

• Проверка форматирования;

• Статический анализ;

• Тесты и динамический анализ;

• Сборка проекта под большинство целевых (Android/Linux arm/aarch64) и хост (Linux/Darwin x64) платформ.

Форматирование кода

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

Для случаев, когда код действительно требует особого форматирования, соответствующие директивы позволяют временно отключить автоматическое форматирование.

Важно проконтролировать, чтобы в CI и локально на машинах разработчиков, использовались одинаковые версии инструментов форматирования, так как результат может отличаться от версии к версии.

Сlang-Format

Утилита clang-format используется для автоматического форматирования C/C++ и Protobuf файлов.

Для выбора подходящего стиля форматирования можно воспользоваться Online конфигураторами, например, Clang-Format configurator, где можно выбрать один из доступных базовых стилей и настроить в соответствии со своими предпочтениями.

Наш стиль основан на LLVM и дополнен специфичными для нашего проекта опциями.
Одной из самых полезных возможностей Сlang-Format является настройка IncludeCategories — порядка включения заголовочных файлов и их сортировка.

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

IncludeCategories:
  # Headers in <> without extension.
  - Regex:           '<([-A-Za-z0-9\Q/-_\E])+>'
    Priority:        4
  # Headers in <> from specific external libraries.
  - Regex:           '<(gtest|gmock)\/'
    Priority:        3
  # Headers in <> with extension.
  - Regex:           '<([-A-Za-z0-9.\Q/-_\E])+>'
    Priority:        2
  # Headers in "" with extension.
  - Regex:           '"([-A-Za-z0-9.\Q/-_\E])+"'
    Priority:        1
IncludeIsMainRegex: '(Test)?$'

Cmake-Format

Утилита cmake-format очень похожа на clang-format, только предназначена для форматирования файлов сборочной системы CMake.

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

    "add_proto_library": {
      "pargs": 1,
      "flags": ["MODULES", "STATIC", "SHARED"],
      "kwargs": {
        "PROTOS": '+',
        "OUT_DIR": 1,
      }
    },

JSON format c помощью jq

Для форматирования JSON-файлов мы используем универсальную утилиту для работы с JSON — jq. Она позволяет отсортировать ключи в алфавитном порядке и настроить одинаковые отступы.

jq -M -S --indent 4 .

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

Статический анализ кода

В качестве инструмента статического анализа мы используем Clang-Tidy. Выбрали этот инструмент, потому что он широко распространен, интегрируется в используемые разработчиками в команде IDE (CLion, Visual Studio Code, Qt Creator) и позволяет:

• находить ошибки (bugprone-, clang-analyzer-);

• проверять стиль кодирования (modernize-, cppcoreguidelines-);

• проверять соответствие Google C++ style guide (google-);

• находить некоторые проблемы в производительности (performance-).

Clang-Tidy продолжает развиваться, в новых версиях появляются новые полезные проверки, такие как misc-include-cleaner для проверки неиспользуемых или отсутствующих #include директив.

Также имеется возможность расширить набор готовых проверок и написать свои собственные.

Quality gate

При внедрении статического анализа в процесс разработки, возникает важный вопрос, как внедрить статический анализ в процесс непрерывной интеграции в качестве quality gate, чтобы сделать исправление предупреждений обязательным? Как при этом не заблокировать разработку?

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

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

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

Мы выбрали подход, основанный на принципе zero-warning policy, но на уровне отдельных компонентов проекта.

Общие принципы:

• Нужно поддерживать нулевое количество предупреждений в проекте (zero-warning policy), чтобы можно было оперативно находить и исправлять новые предупреждения.

• Так как единовременно сложно исправить все предупреждения, то политика zero-warning применяется на уровне отдельных компонентов.

• Используется единый конфигурационный файл Clang-Tidy на уровне проекта - .clang-tidy.

• К коду тестов применяются те же требования, что и для основного кода.

В CI, с помощью основного конфигурационного файла .clang-tidy, проверяются:

• Все новые файлы, добавляемые в репозиторий проекта.

• Файлы компонентов, для которых уже исправлены предупреждения и выполнено условие zero-warning. Соответствующие директории перечислены в виде регулярных выражений в разделе HeaderFilterRegex конфигурационного файла в алфавитном порядке.

Все остальные файлы проверяются только на исправление подмножества критических предупреждений, перечисленных в отдельном конфигурационном файле .clang-tidy-simple. Это позволяет сделать обязательными для всего проекта ряд наиболее важных проверок, таких как bugprone-use-after-move, до того, как будут исправлены все предупреждения в соответствующих компонентах.

Настройка конфигурационного файла

Второй вопрос заключается в том, какие предупреждения нужно включить в конкретном проекте?

Существует два подхода:

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

• Второй - в том, чтобы постепенно включать только действительно нужные предупреждения.

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

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

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

Фрагмент текущего основного конфигурационного файла .clang-tidy:

Checks: '-*,
android-*,
bugprone-*,-bugprone-easily-swappable-parameters,-bugprone-unchecked-optional-access,
clang-analyzer-*,
google-*,-google-build-using-namespace,-google-readability-avoid-underscore-in-googletest-name,
modernize-*,-modernize-use-trailing-return-type,
performance-*,
portability-*,
readability-*,-readability-else-after-return,-readability-magic-numbers,-readability-identifier-length,
misc-*,-misc-non-private-member-variables-in-classes,-misc-const-correctness,-misc-confusable-identifiers,
cppcoreguidelines-pro-type-member-init,cppcoreguidelines-pro-type-const-cast'

HeaderFilterRegex: "\
AssistantSDK/|\
SmartHomeSDK/|\
libs/audio/|\
libs/cpp-common/|\
...

Некоторые проверки выключены, потому что они работают слишком медленно, например, misc-confusable-identifiers или приводят к зависанию Clang-Tidy (на версии 15), например, bugprone-unchecked-optional-access. Последнюю проверку мы не стали выключать полностью, потому что она действительно находит проблемы в нашем коде, поэтому вынесли ее в отдельный шаг, который автоматически перезапускается, если из-за зависания не успевает отработать за отведенный таймаут.

Другие проверки пришлось выключить не потому, что они плохо работают или бесполезны, а потому, что их устранение займет слишком много времени для нашего проекта. Например, google-build-using-namespace и readability-magic-numbers.

Для того, чтобы Clang-Tidy правильно работал, необходимо передать файл c опциями компиляции проекта — compile_commands.json. Наш кроссплатформенный проект содержит модули, специфичные для определенных платформ и устройств. Поэтому, если некоторые модули не компилируется в конфигурации, которая используется для проверки Clang-Tidy, то часть проекта не будет проверена статическим анализом в CI. Мы решаем проблему тем, что в конфигурации по умолчанию собираем весь код, который может собираться на текущей платформе, даже если часть модулей на ней не используется. Это позволяет проверять большую часть проекта используя только одну конфигурацию для статического анализа в CI.

Итак:

• Внедрять статический анализ в процесс разработки лучше на ранней стадии проекта. Большая часть трудностей при внедрении Clang-Tidy у нас была связана с тем, что внедрение происходило в большой проект с трехлетней историей.

• Не нужно бояться выключать предупреждения, которые плохо работают. Clang-Tidy включает большое количество проверок, часть из них может не подходить проекту или его части. Лучше пожертвовать частью проверок, чем откладывать внедрение такого мощного инструмента.

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

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

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

Unit-тесты и динамический анализ

Мы, как разработчики, в первую очередь сфокусированы на Unit-тестах.
В нашем проекте сейчас около 2800 unit тестов и это количество постоянно растет.

В качестве тестового фреймворка используем Google Test и входящий в него Google Mock.

Команды, разрабатывающие алгоритмы обработки звука (Spotter, Voice Quality Enhancement), также пишут performance-тесты c использованием Google benchmark.

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

А еще мы запускаем тесты непосредственно на целевых устройствах, но пока этот этап не интегрирован полноценно в процесс разработки.

Динамический анализ

Для нахождения гонок при обращении к данным (data race) мы запускаем тесты с использованием Google Thread Sanitizer.

Valgrind дополняет Tread Sanitizer в части поиска ошибок синхронизации и также позволяет находить ошибки при работе с памятью.

К сожалению, на практике, большое количество проблем найденных в тестах динамическим анализом с помощью Google Thread Sanitizer и Valgrind относятся к коду теста, а не к самому тестируемому коду. Но, так как эти случаи невозможно разделить заранее, приходится исправлять все найденные проблемы и/или подавлять ложные срабатывания анализатора.

Нестабильные (Flaky) тесты

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

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

Для того, чтобы сделать сами тесты более стабильными мы:

• Используем Stub-ы и Mock-и для изоляции окружения.

• Умножаем количество итераций при прогоне затронутых тестов в CI.

• Стараемся избегать использования определенных таймаутов в тестах.

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

Ночные прогоны

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

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

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

Итак:

• Нужно отслеживать нестабильные тесты. С ростом количества тестов необходим процесс по работе с нестабильными тестами. Рост количества нестабильных тестов может затормозить разработку новых фичей

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

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

Заключение

Как видите, разработка умных устройств — это не только «хардкорный embedded». Разработка приложений для девайсов в том числе ведется на современном C++ с использованием инструментов статического и динамического анализа кода.

Clang-Tidy стал настоящим помощником в нашем проекте, но важно полноценно встроить его в процесс разработки и настроить под требования конкретного проекта.

А какие инструменты используете вы? Делитесь в комментариях!