Есть четыре вещи, заниматься которыми можно бесконечно. И если про первые три все слышали, то четвертая — это смотреть код. Мы с вами немного погрузились в это увлекательное занятие с помощью теста, а теперь покажем, как мы просматриваем код в собственных программах и ищем в них баги. Мы попросили наших разработчиков вспомнить истории на эту тему и рассказать, с чем они сталкивались, как работают сами над качеством кода и как оценивают его у будущих коллег.
Ошибки — это нормально, так как их совершают все, ненормально — когда они попадают в продакт. Чтобы этого не происходило, мы в своих командах постоянно проверяем продукты перед выходом и выстраиваем QA-процесс. В Mail.ru Group много разных команд, и у каждой есть свой опыт, подход и куча интересных историй про то, как что-то где-то пошло (или могло пойти) не так. Мы собрали пять из них, и каждая покажет свою важную составляющую работы над качеством кода.
Не только код-ревью
Привет, Хабр! Меня зовут Марк Локшин, я старший программист IT Territory / MY. GAMES. Разработкой java-серверов для компьютерных игр занимаюсь уже 8 лет и не единожды что-нибудь ломал, но ещё больше раз чинил и предотвращал поломки.
Один запрос к разным БД
Один раз во время подготовки выпуска обновления игры нужно было сконвертировать данные. Стандартно мы делали это с помощью SQL-запроса. Запросы были нужны в синтаксисе MySQL и PostgreSQL. В MySQL синтаксис позволяет указать после UPDATE несколько таблиц, которые можно использовать далее в запросе. PostgreSQL такого не позволяет, у него другой синтаксис. Упрощённо мой запрос выглядел следующим образом (для краткости я убрал связи с другими таблицами):
update test set id = t2.id + 1 from test t2;
update test set id = t2.id + 1 from test t2;
Этот запрос делал совсем не то, что задумывалось. Если вставить в id числа от 1 до 3, то при выполнении запроса PostgreSQL во всех строках поставил 2 (хотя результат исполнения запроса, вообще говоря, не детерминирован: в любой строке может получиться число от 2 до 4). Подобный запрос в синтаксисе T-SQL (update test set id = id + 1 from test;) будет синтаксически верным и действительно увеличит значение столбца id на 1.
Дело в том, что синтаксис PostgreSQL не позволяет задавать одинаковые имена таблицам в секциях update и from, в то время как T-SQL позволяет указывать имя той же таблицы в from, и это будет именно таблица из update. Возможно, из-за этой особенности разработчик допустил ошибку и она не была найдена в процессе код-ревью.
В ходе проверки большая часть тестовых ситуаций была для MySQL, а исполнение запроса для PostgreSQL оказалось недетерминированным. Ошибку во время проверки конвертирования так и не обнаружили. Возможно, там случайно оказались нужные цифры либо на них не обратили внимания.
Кроме тестовых ситуаций у нас есть процедура автоматизированной проверки обновлённой базы данных: после преобразования программа подключается к новой версии и заходит в игру разными игроками. Как только её запустили, сразу заметили, что конвертирование базы стало идти дольше, чем обычно. Так мы и обнаружили проблемный запрос.
Как мы выстраиваем процесс код-ревью
Этот баг мы поймали благодаря налаженному процессу тестирования. Если бы не нашли этот запрос, часть данных у некоторых игроков изменилась бы случайным образом. Причём, скорее всего, это бы выяснилось через определённое время после миграции базы, когда исправить ошибку было бы уже сложнее.
Чтобы избегать подобных ситуаций, мы проводим код-ревью поэтапно и отслеживаем несколько критериев качества. Во-первых, отслеживаем соответствие кода Code Conventions внутри команды. Проверяем, что в коде нет выделенного нами набора инспекций из IDEA и копипасты чужого кода (хотя это и не всегда плохо, просто вместе с кодом нередко копипастят и ошибки). Оцениваем покрытие кода тестами. 100 %-го покрытия кода тестами, конечно, нет, но какой-то хотя бы минимальный набор на каждый функционал быть должен. Как показал данный случай, ошибка может пройти несколько стадий проверки, поэтому в работе важно использовать различные методы поиска ошибок и тестирования приложений.
У новых сотрудников мы оцениваем качество их кода и стараемся сразу подключить к процессу код-ревью. Если это не начинающий разработчик, то через непродолжительное время по мере знакомства с проектом он сам начинает проводить ревью чужого кода. Начинающему разработчику требуется больше времени, чтобы включиться в процесс с обеих сторон. Но по мере накопления опыта и уменьшения количества дефектов в коде стараемся подключать к процессу осуществления ревью как можно раньше.
Регламенты и информирование
Привет, Хабр! Меня зовут Анна Долгова, и я прошла путь от тестировщика до тимлида группы тестирования фронтенда в проекте «Пульс» — рекомендательной системе, которая работает на базе технологий машинного обучения и предлагает пользователям контент и рекламу с учётом их интересов.
Что может непроверенный конфиг
На второй неделе работы мне поручили проверить релиз админки, откуда фронтенд-демон для рекомендаций забирает настройки, на основании чего система определяет, что будет показываться пользователям. Я проверила все задачи и написала в общий чат, что можно выкатываться.
Релиз выкатили, а мы все пошли на daily scrum meeting, где по алертам из Grafana узнали, что пользователям перестала показываться реклама. Мы откатились к прошлой версии админки и стали разбираться. Так как я работала недавно, то не знала, что у админки есть конфиг, который она выгружает в наш фронтенд-демон для рекомендаций. А именно в тот раз конфиг админки отгружал невалидные поля, которые не смог прочитать демон.
Полная проверка по готовым регламентам
Чтобы такого не повторилось, мы:
- обязательно после проверки всех задач в релизе проверяем, что конфиг не поломается;
- написали регламент проверки конфигураций;
- обязательно после проверки всех задач в релизе проверяем совместимость текущей версии фронтенд-демона для рекомендаций и новой версии админки;
- написали регламент проверки совместимости текущей версии фронтенд-демона для рекомендаций и новой версии админки;
- если у нас готовится новый релиз фронтенд-демона, проверяем релиз админки сразу с двумя версиями;
- попросили разработчиков написать регламент по выгрузке полей и их содержанию;
попросили разработчиков тщательнее покрывать код unit-тестами.
Новые сотрудники в моей команде должны иметь общее представление о составных частях проекта, а также детальное представление о тех частях, которые им будет необходимо тестировать. Для этого в confluence собрана вся информация и инструкции, которые мы постоянно обновляем и дополняем. Мы регулярно проводим встречи, где делимся друг с другом опытом. В среднем новый сотрудник полностью погружается в работу за три месяца.
Растущие объёмы кода
Привет, Хабр. Меня зовут Станислав Барбуков, и я работаю руководителем отдела тестирования в Pixonic. Я сам подбираю кадры для своей команды, обучаю их и оптимизирую тестирование в нашей компании.
Как тестирование тормозит разработку
В Pixonic мы оцениваем качество кода с помощью метрики «плотность дефектов». За 2019 и 2020 годы она составляет 7−9 ошибок на 1000 строк кода. Проблема в том, что даже при стабильной плотности дефектов объёмы кода постоянно растут: так, в прошлом году команда тестирования находила от 500 до 1200 багов в основном проекте за месяц.
В итоге команду тестирования необходимо было регулярно наращивать: бюджет рос, а релизы выходили реже. Чтобы снизить нагрузку на ребят и быстрее выкатывать новые версии игры, я переработал наш подход к тестированию и внедрил новую систему работы.
Ускорение тестирования
Я решил использовать более продвинутый подход, который сократит рутинные операции и ускорит тестирование:
- ввёл краткие чек-листы вместо объёмных тест-комплектов;
- сфокусировал команду на основных «пользовательских сценариях» использования приложения вместо большого количества избыточных «негативных сценариев»;
- отказался от классических огромных регрессионных сессий проверок версии перед релизом и автоматизировал тестирование;
- результат: меньше временных затрат на тестирование, отсутствие потребности в большем количестве сотрудников, сокращение сроков тестирования;
- использовал сильные стороны сотрудников там, где это было максимально целесообразно и эффективно.
Внутри компании у каждого проекта есть свой приоритет, по которому мы выбираем степень рисков, которую мы можем позволить себе при тестировании. Грубо говоря, чем лучше финансовые показатели у проекта, тем тщательнее мы его проверяем. Например, на проекте War Robots мы работаем по самой безопасной методологии: проверяем по чек-листам, оставили минимально необходимые негативные сценарии, регрессионные сессии составляем на основе конкретных изменений в проекте, всю рутину отдали на откуп автотестам, а на самые критичные истории назначаем только проверенных и опытных инженеров.
На ранних этапах разработки проектов цена ошибки не столь высока и такие процессуальные манипуляции помогают существенно экономить время и ресурсы. Автоматизация весьма недешевое удовольствие, а в случае с играми готовых решений на рынке очень мало и приходится использовать собственное решение. Мы разрабатывали собственный фреймворк для автоматизации 3 года, из которых полтора ушло на его полишинг и написание самих тестов. При этом тестовое покрытие проекта ещё далеко не 100%-е и находится в активной разработке.
Когда мы принимаем новых сотрудников, на этапе собеседования приблизительно оцениваем опыт соискателя и сопоставляем его схожесть с процессами внутри Pixonic. В полном объёме оценка компетенций сотрудника проходит в течение первого года сотрудничества. Мы назначаем из более опытных сотрудников ментора, планомерно поручаем всё более сложные задачи и следим за количеством дефектов после поставки. Параллельно проводим обязательную сертификацию по методологии ISTQB, внутренние обучения.
Работа с dev-контуром
Привет, Хабр! Меня зовут Иван Решетин. Последние 1,5 года я руковожу бэкенд-платформой в сервисе объявлений Юла. Помогаю внедрять микросервисы, улучшать инфраструктуру и делать Юлу быстрее, выше, сильнее. Люблю A/B-тесты и поэтапное внедрение.
Ошибка в одну строчку
Как-то раз нам нужно было реализовать небольшую задачку: добавить сохранение истории изменений в админке A/B-тестов. После выкатки в продакшен первое же изменение одного из A/B-тестов затерло всю базу, в которой было более 200 кейсов со сложными конфигурационными настройками. Начали разбираться, какая последняя задача была и как она выкатывалась. В той задаче в легаси-код была добавлена практически одна строчка нового функционала. Но вместе с этим решили поправить немного кодстайл, а также для лучшего понимания кода изменили имя у переменной, которая содержит новое значение A/B-теста. В итоге пулреквест (схематично) выглядел так:
Мы используем MongoDB как основное хранилище. А модуль A/B-тестов был очень старый и все A/B-тесты хранились в одном (!) документе. Выглядело это как-то так:
{
"_id" : ObjectId("5acf0c97640f499c8cd8dc09"),
"code" : "module_ab_tests",
"data" : {
"abtest_name1" : {<config_data>}
"abtest_name2" : {<config_data>}
"abtest_name3" : {<config_data>}
}
}
Пазл сложился. Изначально в переменной $config хранился map со всеми A/B-тестами. И так как новая переменная $configAbModule была пустая, то в результате маленькая правка меняла data, оставляя в нём только ключ с текущим A/B-тестом. На код-ревью этот момент пропустили, а автотесты проверяли только сохранение конкретного теста: изменил, сохранил, получил… всё ок… но проверки остальных значений A/B-тестов не было.
Ещё одной причиной того, что удаление данных из старой базы в dev-контуре никто не заметил, стала подготовка к запуску нового сервиса A/B-тестов на Go. Он уже имел отдельное хранилище и использовался в dev-контуре последние несколько недель. Этот сервис также был раскатан в бою, но пока без боевой нагрузки. Поэтому мы переключились на него, но на 2 дня раньше запланированной даты. Иначе нам пришлось бы экстренно раскатывать 11 шардов из бэкапов.
Внимание к деталям
Этот случай научил нас уделять внимание даже «однострочным» правкам, а ещё мы расширили тест-кейсы и начали проверять всё, что могло зааффектить. Также мы стали делать dev- и prod-окружение более похожим и регулярно проводить учения по развертыванию бэкапов.
Чтобы все ребята в команде понимали, как устроен продукт, мы проводим внутренние митапы по архитектуре, где рассказываем, как устроены различные компоненты. Во время онбординга показываем записи со всех митапов новичкам, чтобы они лучше понимали, как устроен продукт, и проще втягивались в работу.
Внимание к задачам на этапе разработки
Привет, Хабр! Меня зовут Стас Фомин, и я отвечаю за тестирование проектов студии NORD. Мы одна из старейших студий MY. GAMES: с 2007 года занимаемся разработкой и продюсированием онлайн-игр. Да, это мы сделали Jungle Heat, Iron Desert и Hustle Castle.
К чему приводит усложнение
Представьте себе, что у юнитов в бою перестанет отображаться полоска здоровья (HP). Кому-то покажется мелочью, но для геймеров это очень важная часть игрового процесса, и её поломка грозила превратиться в огромное число тикетов от саппорта и потерю аудитории.
Проблема оказалась довольно сложной. Она возникла из-за неправильной концепции кода отображения HP в графическом интерфейсе пользователя (GUI). Мы отправляли не конкретное значение HP в текущий момент, а дельту — показатель, как изменился HP после определённого действия. Уже внутри UI из этой дельты рассчитывались значения текущего и максимального здоровья. Это совсем атас — фактически HP для боёвки и HP для UI считались в разных местах, а максимальное и текущее HP отправлялись в GUI отдельно друг от друга. В результате нам пришлось переписать визуализацию изменений HP и логику применения баффов на HP.
Как мы боремся с узкими местами
Чтобы избежать подобных багов, мы выделили для себя критичные участки кода. Так боевая система на проектах полностью покрыта автотестами, которые мы постоянно пополняем. Также у нас есть клиентские и серверные unit-тесты, покрывающие самые критичные участки кода. Есть отдельные тестовые сценарии для проверки клиент-серверного взаимодействия, игровой логики. Все тесты крутятся в CI Jenkins и запускаются каждую ночь на регулярной основе. Код-ревью клиентской и серверной части проводят ведущие разработчики на проекте по каждой фиче.
Задачи идут с подробным описанием логики реализации и узких мест, которые продумываются заранее, ещё на этапе проработки концепции. Для валидаций нейминга и структуры статичных файлов (графика/видео/звуки) используются commit hooks, которые показали себя довольно эффективно.
Так мы добиваемся того, что большая часть задач становится очевидной даже для новых сотрудников. Период адаптации новых сотрудников у нас составляет 3 месяца: за это время они изучают наши рабочие процессы, задачи на проекте, знакомятся с командой. Для этого есть подробные гайды, FAQ, а наставник помогает вникнуть в технические детали проекта.
Качественный код — это искусство. У этого искусства две стороны. Одна — налаженный процесс тестирования, умелое введение в курс дела новобранцев, код-ревью, митапы по архитектуре, подробное описание задач, продумывание логики реализации и узких мест. И это далеко не все существующие способы делать код ещё лучше. Вторая — непрерывная работа команды над собственными скилами. В MRG мы сосредоточены на обоих процессах и делаем всё, чтобы мягко погрузить нового сотрудника в работу и помогать ему совершенствоваться. Хочешь стать частью нашей команды? Смотри наши вакансии или заполни анкету: