Как стать автором
Обновить

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

Python элегантный язык, приятно изучать и пользоваться :)
по количеству минусов так и не скажешь

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


Итак, по порядку:


Он предоставляет готовые механизмы, чтобы...

Не тянет тезисной описание на список фич фреймворка. Не очень понятно зачем он нужен как отдельная сущность, а не как горсть плагинов для пайтеста или как отдельный мок-сервис. Вообще, солянка какая-то: первый тезис описывает http-клиент, второй мок-сервис, в третьем все про тот же мок-сервис(?), всё что написано в четвёртом — можно сделать yield фикстурами пайтеста.


Готовых инструментов для этого нет — вам пришлось бы писать код для настройки тестового окружения, который будет… "запускать в этом окружении тестируемый сервис."

А зачем вам вообще это надо? Кстати, вы базу локально запускаете? Почему не запустить через докер и не залить фикстуркой? Докер пинать, к слову, можно тоже из питона.


Решать эту задачу, пользуясь фреймворками для unit-тестов, слишком трудно и неправильно

Вот тут надо уточнять, что вы вкладываете в эти понятия и почему неправильно.


Можно запускать и на каждый тест (убрать scope='session'), но это медленно

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


os.getenv('PYTHON3', 'python3')

Что это за чудо-переменная?


    python_path = os.getenv('PYTHON3', 'python3')
    service_path = pathlib.Path(__file__).parent.parent

Это грязь. Я бы передавал путь к приложения через переменную окружения.


Команда запуска сервиса. Первый элемент массива — исполняемый файл,

Во-первых, не массива, а списка. Во-вторых, крайне не рекомендую такой способ инициализации чего-либо, если допустимо использование с питоном версии меньшей чем 3,7.


service_daemon,, HTTP-статус ответа == 204

В примере, вставленном в статью синтаксическая ошибка


Обратите внимание, в тестах chat-backend мы никак не используем сервисы хранилища, ни chat-storage-mongo, ни chat-storage-postgres. Чтобы chat-backend нормально обработал вызовы, мы мокаем API хранилища с помощью mockserver.

Погодите! В начале же обещали, что тесты смогут "поднимать и наливать базу данных".


tests_dir = pathlib.Path(file).parent
sqldata_path = tests_dir.joinpath('../schemas/postgresql').

Грязь. Для примера кода к статье не допустимо. Вам бы завести какой, прости господи, файлик settings.py и в него поскладывать константы с путями и пр.


Проверим, что в теле ответа JSON с идентификатором сохранённого сообщения

Напрашивается валидация тела ответа через marshmallow.


Запускать примеры легче всего в докер-контейнере.

Погодите, так вы все таки умеете готовить докер и все работает через контейнеры? А как же "запускать в этом окружении тестируемый сервис". Так все таки все работает через контейнеры или в одном окружении? Из статьи крайне не ясно.


make run-chat-mongo

Я вас умоляю… Зачем пинать композ через мэйк?


По коду:


  • асинхронщина в тестах всегда должна быть оправдана. Зачем оно вам, и тем более, в демонстрационном примере для статьи?
  • по коду куча констант и в том числе строковых. Magic number же?
  • не используется typing от слова совсем. В случае с пайтестом, который любит передавать фикстуры аргументом функции тайп хинтинг сильно облегчает жизнь и позволяет дышать полной грудью.
  • очень много работы с относительными путями, что само по себе не айс
  • обычный assert  очень не информативен — к нему в таких тестах надо либо подписывать сообщение, либо использовать какой-либо матчер вроде hamcrest
  • субъективно, коду примера не хватает хорошего ревью
Благодарю за подробный комментарий!

асинхронщина в тестах всегда должна быть оправдана. Зачем оно вам, и тем более, в демонстрационном примере для статьи?

Асинхронные обработчики нужны, например, чтобы моксервер отвечал на запросы, пока тест ожидает ответ сервиса, без асихнронности никак.

по коду куча констант и в том числе строковых. Magic number же?

Бывает, особенно часто в предусловиях и проверках, что код оказывается проще и понятнее, если не выделять константу.

очень много работы с относительными путями, что само по себе не айс

Согласен, но это необходимо написать только один раз. Обычно мы используем кодогенерацию и пути подставляются на этапе cmake.

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

Действительно, значительная часть кода ещё не покрыта аннотациями, это наследие тех времён, когда проект работал на python2.7/gevent. Мы постепенно добавляем аннотации в имеющийся код и используем их в новом.

* обычный assert очень не информативен — к нему в таких тестах надо либо подписывать сообщение, либо использовать какой-либо матчер вроде hamcrest

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

Погодите! В начале же обещали, что тесты смогут «поднимать и наливать базу данных»

Сервис chat-backend не взаимодействует с базой, за это отвечает другой сервис, chat-storage-postgres, вот когда мы тестируем chat-storage-postgres, тогда и наливаем базу.

Погодите, так вы все таки умеете готовить докер и все работает через контейнеры? А как же «запускать в этом окружении тестируемый сервис». Так все таки все работает через контейнеры или в одном окружении? Из статьи крайне не ясно.

Действительно, testuite может поднимать базу самостоятельно и «запускать в этом окружении тестируемый сервис», а может воспользоваться и готовым окружением, в том числе поднятым в docker, что удобно в CI. Мы в Яндекс.Такси используем оба решения.

Не тянет тезисной описание на список фич фреймворка. Не очень понятно зачем он нужен как отдельная сущность, а не как горсть плагинов для пайтеста или как отдельный мок-сервис. Вообще, солянка какая-то: первый тезис описывает http-клиент, второй мок-сервис, в третьем все про тот же мок-сервис(?), всё что написано в четвёртом — можно сделать yield фикстурами пайтеста.

Это введение, мы стремимся дать примерное представление, более целостное картину дают последующие разделы статьи.

Вот тут надо уточнять, что вы вкладываете в эти понятия и почему неправильно.

Потому что они прендазначены для решения другого типа задач. Статья про testsuite, а не юнит тесты, поэтому здесь мы не будем углубляться.

Что это за чудо-переменная?

Тут есть несколько вариантов, как это обойти: можно передавать пути через аргументы, через переменные окружения или вычислять их на ходу. В данном случае мы добивались того, чтоб тесты запусальись без указания дополнительных параметров. PYTHON3 необходима для того, чтобы можно было запускать тесты в окружении отличном от окружения самого testsuite.

А в общем: согласен, что не очень красиво — поправим.

Во-первых, не массива, а списка. Во-вторых, крайне не рекомендую такой способ инициализации чего-либо, если допустимо использование с питоном версии меньшей чем 3,7.

В самом деле, list :)

В примере, вставленном в статью синтаксическая ошибка

Спасибо, исправили.

Я вас умоляю… Зачем пинать композ через мэйк?

С помощью make-файла мы показываем, как можно запустить нужные команды, мы не настаиваем на использование make.

Напрашивается валидация тела ответа через marshmallow.

Мы стремимся, насколько возможно, не навязывать пользователю выбор инструментов, поэтому testsuite предоставляет базовую функциональность. Поддержку валидации, скажем через marshmallow, можно добавить в своем проекте. Например, в Яндекс.Такси все ручки описаны с помощью yaml-схем, поэтому дополнительная валидация нам в этом месте не очень полезна.
Асинхронные обработчики нужны, например, чтобы моксервер отвечал на запросы, пока тест ожидает ответ сервиса, без асихнронности никак.

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


Бывает, особенно часто в предусловиях и проверках, что код оказывается проще и понятнее, если не выделять константу.

В таком количестве как у вас оно не бывает оправданным. Магическое значение есть магическое значение, тем более когда их много и они разбросаны по коду. Если вы считаете, что в тестах можно говнокодить в отличии от боевого кода, ну что уж тогда вообще разговаривать.


Согласен, но это необходимо написать только один раз. Обычно мы используем кодогенерацию и пути подставляются на этапе cmake.

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


Например, в Яндекс.Такси все ручки описаны с помощью yaml-схем, поэтому дополнительная валидация нам в этом месте не очень полезна.

У вас доверительное тестирование чтоль? А если разработчик налажает в схеме и допустит ошибку?

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

Без async / await не обойтись в коде тестов. Если под везде подразумевается что-то ещё кроме кода тестов, то нет, не везде. В частности, сервис не обязан использовать async, а может вообще быть написан на другом языке, скажем C++.

В таком количестве как у вас оно не бывает оправданным. Магическое значение есть магическое значение

Я готов согласиться, что где-то магические строки / числа по коду не нужны, но, ещё раз, это верно не всегда. Часто бывает так, пишешь вначале тест с константами, получается лаконично, но сходу не понятно, что проверяется. Заменяешь константы на значения, и стало понятнее.

У вас доверительное тестирование чтоль? А если разработчик налажает в схеме и допустит ошибку?

Конечно нет, разработчик должен выписать ожидаемый в ответе json отдельно, но речь о другом. Мы не навязываем конкретный способ, которым это будет сделано. Можно просто сравнить dict в питоне, можно прикрутить фреймворк на ваш вкус.
Без async / await не обойтись в коде тестов

Приехали

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


По коду:


Если уж вы его причесали форматером с дефолтными настройками, а судя по всему это было что-то вроде yapf -i -r yandex-taxi-testsuite/testsuite, то надо было хотя бы допустить размер строки до 120 символов, дабы не плодились попусту уродские переносы строк.


В коде очень часто встречается странное и с этим надо бороться. Глазам регулярно больно смотреть. Например такое:


           try:
                decoded = line.decode('utf-8')
            except UnicodeDecodeError:
                logger.error(
                    'Failed to decode subprocess output', with_exc=True,
                )
                continue

Вот так делать нельзя в публичном коде, да и в обычном. Пролюбите строчку какую важную. Сделайте line.decode('utf-8', errors='replace') и удалите вон того Франкенштеина, что выше. Из кода также надо вычистить всякие дефолты в ноль для значения портов (как это вообще возможно?) и отрефачить код. Вообще, все те же магические значения повсюду, плоховатая организация кода, не указан тайп хинтинг даже для возвращаемых данных фикстур. Некоторые вещи можно было бы обмотать аллюровыми украшательствами раз уж он прижился у всех в тестировании. Проект забахан совсем недавно и одним коммитом => не ясна его дальнейшая судьба.


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

Если уж вы его причесали форматером с дефолтными настройками, а судя по всему это было что-то вроде yapf -i -r yandex-taxi-testsuite/testsuite, то надо было хотя бы допустить размер строки до 120 символов, дабы не плодились попусту уродские переносы строк.

Мы форматируем код по pep8, с длиной строки в 79 символов.

Вот так делать нельзя в публичном коде, да и в обычном. Пролюбите строчку какую важную. Сделайте line.decode('utf-8', errors='replace') и удалите вон того Франкенштеина, что выше.

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

Из кода также надо вычистить всякие дефолты в ноль для значения портов (как это вообще возможно?) и отрефачить код.

0 означает, что операционная система выберет любой порт. Это стандартное значение для bind(2).
Проект забахан совсем недавно и одним коммитом => не ясна его дальнейшая судьба

Но как публичный проект — громоздкий, ...

Проект действительно enterprise, появился и развивается решая конкретные задачи. Это верно сейчас, это будет верно и в будущем, когда проект будет и декомпозирован, и почищен, и так далее, будем заниматься этим, обязательно.
Мы форматируем код по pep8, с длиной строки в 79 символов.

Это походу самое часто игнорируемое правило. Вам самим-то приятно на такое смотреть:


logger.error(
                    'Failed to decode subprocess output', with_exc=True,
                )

Плюс форматеры тоже надо настраивать чтоб таких не выходило:


    def __init__(
            self,
            base_url,
            *,
            mocked_time,
            span_id_header=None,
            raw_response=False,
            **kwargs,
    ):

?


Если уж вы договорились, что строго по пепе живете, то и код надо писать компактненько, а не отдавать все на откуп форматеру.


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

Действительно, при UnicodeDecodeError ведь столько вариантов проблем...


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

Ох, ребят )) Здесь койки двигать уж себе дороже будет.

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

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

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

Не согласен. Через задницу настроенный форматер это не беда инструмента, а изъян команды, боль глазам, (в данном случае) куча избыточных строк и как результат — низкое качество кода.

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

logger.error(
                    'Failed to decode subprocess output', with_exc=True,
                )


они не делают, это был просто кривой копипаст, в действительности бывает такое

logger.error(
    'Failed to decode subprocess output', with_exc=True,
)

Главная претензия ни к кривым скобкам была, а в общем-то в том, что не понятно зачем простой крохотный лог занимает три строки кода.

применяется в Яндекс.Такси


Интересно что используют другие сервисы Яндекса, и почему не этот фреймворк? Не подошло?
Помимо Яндекс.Такси, мы используем testsuite при разработке некоторых сервисов Яндекс.Лавки. Основной потребитель всё ещё Яндекс.Такси, потому что именно в Такси разработали testsuite, мы изначально используем его для всех сервисов.

Основа — pytest. Молодцы, никаких велосипедов там, где не надо.


… С точки зрения эстетики (я поверхностно смотрю) — оно не выглядит сильно приятнее, чем (условный) betacam с кассетами ответов. Почему так много и так некрасиво?

Не понял, это был риторический вопрос или что?

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

В контексте некоторой глубины, при которой вопрос не риторический, не могли бы вы уточнить, чего много и некрасиво?
Будет ли статья на тему «Интеграционные тесты testsuite»?
Таких планов не было, но идея интересная, подумаем.
йоу!
а можешь рассказать насколько производительный фреймворк и какое кол-во запросов в секунду удавалось достичь в пике?
assert 'id' in data
возможно я не прав, но мне кажется правильнее
assert data['id']
В Вашем случае тест пройдет, если в json есть id, но он пуст.

согласен

Зарегистрируйтесь на Хабре, чтобы оставить комментарий