Всем привет!
Меня зовут Антон и сейчас (не так долго, около года) я разрабатываю на PHP в одном большом и старом проекте. Для обеспечения качества проекта мы применяем автотесты на фреймворке PHPUnit. Но, к сожалению, так получилось, что большая часть наших автотестов функциональные. Недавно мы поняли, что если мы продолжим использовать автотесты прежним образом, то скоро время, потраченное на решение проблем с их не прохождением на CI станет больше, чем время, затраченное на написание полезного кода (на самом деле нет, но оно растёт и это не приятно).
Для системного решения этой проблемы мы выбрали подход, который предполагает написание множества unit-тестов и минимального количества фунциональных тестов. Так же мы решили, что в рамках изменений существующего кода необходимого по правилу бойскаута актуализировать существующие тесты. Некоторое время — все это время мы пилили новые фичи — у нас все было прекрасно. Но недавно мне прилетела задача исправить багу в старом коде. Я начал писать unit-тесты, и понял, что проснулось древнее зло легаси. Стало понятно, что это путь в никуда:
- я писал пару тест-кейсов несколько часов;
- понял, что за это время написал очень, очень много повторяющегося кода;
- фактически не сделал полезной работы;
- в случае изменения DI придется потратить очень много времени на бесполезную адаптацию кода теста.
Эта статья о том, как я решил проблему отказа от написания не несущего ценности кода. Под катом — итоговый вид тест кейса, с исключением большей части механического кода
Название статьи отсылает к автоматизации в стиле канбан — с одной стороны мы выполнили автоматизацию, а с другой стороны — все ключевые решения принимаются человеком.
Как стало
<?php declare(strict_types=1);
namespace Company\Bundle\Tests\Unit\Service;
use PHPUnit_Framework_MockObject_MockObject;
/**
* @method \Company\Bundle\Service\TestedService getTestedClass()
* @property PHPUnit_Framework_MockObject_MockObject|\Company\AnotherBundle\Repository\DependencyRepository $dependencyRepository
* @property ConstDependencyInjectionParameter $diConstParam
*/
class TestedServiceTest extends Intelligent_PHPUnit_Framework_TestCase
{
/**
* @throws \Exception
*/
public function test_getData_DataDescripion_ResultDescription(): void
{
// data
$this
->diConstParam
->set("value")
;
// behaviour
$this
->dependencyRepository
->expects($this->once())
->method('findData')
->with(10, 10)
->will($this->returnValue([]))
;
// SUT
$clazz = $this->getTestedClass();
// act
$res = $clazz->getData();
// asserts
}
}
Как было
Было плохо. Была бесконечная спагетти из генерации моков для зависимостей, длиннющий вызов конструктора в тест-кейсе. В общем, было некрасиво, и где-то внизу статьи заметно обрезанный пример такого теста приведен.
Немного боли в наших условиях
К сожалению, наш проект использует PHPUnit 3.x. Да, я знаю что он перестал поддерживаться еще до 2014(!) года, но, к сожалению, мы не можем от него отказаться в локальном масштабе времени.
Решенные проблемы
Проблемы, которые пришлось решать — это следствие большого количества старого кода, который написан в стиле, который не очень удобно покрывать автотестами. Основная проблема кода, которая решалась — это огромное (действительно, очень большое, иногда больше 20) число зависимостей в классах-сервисах, которые, на самом деле, являются первыми кандидатами на покрытие unit-тестами в связи с достаточно большим количеством ветвлений, граничных случаев и циклов.
Анализ показал, что, непосредственно «к ручной генерации» кода приводит:
- Необходимость мокировать все зависимости серсивов в методе setUp.
- Необходимость освобождать память моков методе tearDown. Возможно, это не обязательно, но по нашей конвенции мы это делаем
- Необходимость создавать объект для тестирования в каждом тест-кейсе.
Как проходил поиск решения
Рассмотрим повторяющиеся фрагменты кода.
Генерация моков
Повторяющийся фрагмент кода:
$this->service = $this
->getMockBuilder(Service::class)
->disableOriginalConstructor()
->getMock()
;
У PHP есть хороший кандидат на то, чтобы спрятать монотонную эту монотонную работу — магическая функция:
__get
Но нам еще надо передать тип, который мы мокируем. И тут нашлось подходящее средство — phpDoc:
@property Type $service
В нашем случае, особенно с учетом возможностей современных IDE удобней всего оказалось задать его в следующем формате:
@property \FQCN\Service|\PHPUnit_Framework_MockObject_MockObject $service
Внутри функции __get абстрактного класса Intellegence_PHPUnit_Framework_TestCase происходит метапрограммирование, которое парсит phpDoc и формирует на его основе моки, доступные в виде $this->propertyName в тестовом классе. Внимание, весь приведенный в статье код является немного псевдокодом, полная версия — в репозитории:
abstract class Intelligent_PHPUnit_Framework_TestCase extends PHPUnit_Framework_TestCase {
public function __get(string $name)
{
// propertyBag содержит уже готовые моки сервисов, которые хранятся по ключу имени переменной в phpDoc property
$property = $this->getPropertyBag()->get($name);
if ($property === false) {
throw new Exception("Invalid property name: ${name}");
}
return $property;
}
private function getPropertyBag(): PropertyBag
{
if ($this->propertyBag === null) {
// Парсинг проходит в отдельном классе
$this->propertyBag = (new DocCommentParser())
->parse(
// Рефлексия по извлечению phpDoc происходит по FQCN класса. Мы получаем FQCN родительского класса, в котором и описываются требуемые сервисы
get_class($this),
// Создание мока происходит через callback, поскольку функция getMockBuilder - часть базового класса для unit-тестов
function (string $className): PHPUnit_Framework_MockObject_MockObject {
return $this->createDefaultObjectMock($className);
}
);
}
return $this->propertyBag;
}
}
Как можно заметить, непосредственно парсинг phpDoc вынесен в отдельный класс. Давайте посмотрим на него:
class DocCommentParser
{
/**
* @param string $className
* @param callable $mockCreator
* @throws Exception
* @return PropertyBag
*/
public function parse(
string $className,
callable $mockCreator
): PropertyBag {
/** @var string[] $res */
$r = new ReflectionClass($className);
// Непосредственное извлечение phpDoc
$docComment = $r->getDocComment();
if ($docComment === false) {
return new PropertyBag([]);
}
return $this->parseDocString($docComment, $mockCreator);
}
private function parseDocString(
string $docComment,
callable $mockCreator
): PropertyBag {
/** @var string[] $map */
$map = [];
/** @var string[] $constructorParams */
$constructorParams = [];
// Находим все строки-кандидаты на шаблон: property Type1|Type2 $propertyName
// Осталась именованная группа mock, потому что DI может содержать не только сервисы, но и константы
preg_match_all('/@property\s*(?<mock>(\S*)\|(\S+)\s*\$(\S+))/', $docComment, $matches, PREG_SET_ORDER);
foreach ($matches as $match) {
if (array_key_exists('mock', $match) && !empty($match['mock'])) {
// Если из указанных типов не является моком - это не наш кандидат
// Это дополнительный предохранитель
if ($this->isMockObject($match[2])) {
$map[$match[4]] = $mockCreator($match[3]);
continue;
}
if ($this->isMockObject($match[3])) {
$map[$match[4]] = $mockCreator($match[2]);
continue;
}
}
}
return new PropertyBag($map);
}
}
Чего же мы достигли на данный момент? Из того, что мне не нравится — необходимость использовать FQCN в phpDoc property для клиентских зависимостей. Как это побороть — не знаю, буду благодарен за любые предложения.
Было
class ServiceTest extends PHPUnit_Framework_TestCase
{
/** @var DependencyRepository|PHPUnit_Framework_MockObject_MockBuilder */
private $repository;
protected function setUp()
{
parent::setUp();
$this
->repository = $this->getMockBuilder(DependencyRepository::class)
->disableOriginalConstructor()
->getMock()
;
}
protected function tearDown()
{
$this->repository = null;
parent::tearDown();
}
public function test_test()
{
// $this->repository = ...
}
}
Стало
/** @property PHPUnit_Framework_MockObject_MockObject|\Company\AnotherBundle\Repository\DependencyRepository $repository */
class ServiceTest extends Intelligent_PHPUnit_Framework_TestCase
{
public function test_test()
{
// $this->repository = ...
}
}
Писать одну строчку кода вместо 3-4-5 (зависит от вашей конвенции) — интересно. Если обратить внимание, что даже в хорошем классе, например, 7 зависимостей — здорово. С учетом того, что у меня было по 20 зависимостей — просто праздник какой-то.
Создание экземпляра тестового класса
На данный момент заметно ускорилось создание теста. Ура! Но когда я стал писать тест-кейсы, то понял, что боль все еще со мной.
Пример повторяющегося кода:
public function test_case()
{
// SUT
// Так может быть, поскольку, в принципе через DI можно пробросить динамические объекты.
// Например, значение вызова функции другого объекта DI.
$this->clazz = $this->createTestClass(
$dependency1,
$dependency2,
$dependency3 //, ...
);
}
Фактически, нам нужно вызвать одну и туже функцию с одним и тем же набором параметров во всех тест кейсах. На этот случай у php есть магическая функция __call, а в phpDoc подходящая аннотация method.
Постойте, постойте, но ведь конструктору нужен еще и набор параметров в определенном порядке?! Точно, и фактически мы их задали аннотациями property! Это очень радостный момент, когда в твоем коде возникает такая синергия. Будем считать, что порядок задания property — это порядок следования аргументов конструктора. На мой взгляд — терпимое допущение, на практике оно не сильно мешает.
Итак, мы будем использовать следующие конструкции:
/**
* @method FQCN\TestedService getTestedClass()
*/
class ServiceTest
{
public function test_case()
{
// SUT
$clazz = $this->getTestedClass();
}
}
Давайте посмотрим на реализацию магической функции __call:
abstract class Intelligent_PHPUnit_Framework_TestCase extends PHPUnit_Framework_TestCase
{
private const GET_TESTED_CLASS_METHOD_NAME = 'getTestedClass';
/**
* @param string $method
* @param array $params
* @throws Exception
* @return object
*/
public function __call(string $method, array $params)
{
if ($method !== self::GET_TESTED_CLASS_METHOD_NAME) {
throw new Exception("Invalid method name: ${method}");
}
if (!array_key_exists(self::GET_TESTED_CLASS_METHOD_NAME, $this->getMethodsMap())) {
throw new Exception('Method ' . self::GET_TESTED_CLASS_METHOD_NAME . ' not annotated');
}
// Обратите внимание, что PropertyBag расширился и теперь запоминает порядок следования аннотаций property - порядок следования аргументов конструктора
$params = $this->getPropertyBag()->getConstructorParams();
$paramValues = [];
foreach ($params as $param) {
$property = $this->__get($param);
$paramValues[] = $property;
}
$reflection = new ReflectionClass($this->getMethodsMap()[self::GET_TESTED_CLASS_METHOD_NAME]);
// Через рефлексию получаем экземпляр тестового класса с нужными аргументами
return $reflection->newInstanceArgs($paramValues);
}
}
Как изменился парсер? Теперь формирует помимо моков еще и порядок следования заданных property:
class DocCommentParser
{
/**
* @param string $className
* @param callable $mockCreator
* @throws Exception
* @return PropertyBag
*/
public function parse(
string $className,
callable $mockCreator
): PropertyBag {
/** @var string[] $res */
$r = new ReflectionClass($className);
$docComment = $r->getDocComment();
if ($docComment === false) {
// Передаем пустой порядок следования аргументов
return new PropertyBag([], []);
}
return $this->parseDocString($docComment, $mockCreator);
}
private function parseDocString(
string $docComment,
callable $mockCreator
): PropertyBag {
/** @var string[] $map */
$map = [];
/** @var string[] $constructorParams */
$constructorParams = [];
preg_match_all('/@property\s*(?<mock>(\S*)\|(\S+)\s*\$(\S+))/', $docComment, $matches, PREG_SET_ORDER);
foreach ($matches as $match) {
if (array_key_exists('mock', $match) && !empty($match['mock'])) {
if ($this->isMockObject($match[2])) {
$map[$match[4]] = $mockCreator($match[3]);
// Запоминаем порядок следования аргументов
$constructorParams[] = $match[4];
continue;
}
if ($this->isMockObject($match[3])) {
$map[$match[4]] = $mockCreator($match[2]);
// Запоминаем порядок следования аргументов
$constructorParams[] = $match[4];
continue;
}
}
}
return new PropertyBag($map, $constructorParams);
}
}
Прямо сейчас мы избавились от необходимости писать однотипные вызовы по генерации моков и однотипные вызовы по формированию тестового класса. Суммарно, это достаточно заметное число строк кода. Даже если считать, что в класс через DI пробрасываются только статически-мокируемые параметры, то есть, в тесте можно написать приватный метод без параметров, который вернет тестируемый класс — это все равно некоторое улучшение, которое позволяет отказаться от написания бесполезного кода
Было
class ServiceTest extends PHPUnit_Framework_TestCase
{
/** @var DependencyRepository|PHPUnit_Framework_MockObject_MockBuilder */
private $repository;
protected function setUp()
{
parent::setUp();
$this
->repository = $this->getMockBuilder(DependencyRepository::class)
->disableOriginalConstructor()
->getMock()
;
}
protected function tearDown()
{
$this->repository = null;
parent::tearDown();
}
public function test_test()
{
$this->clazz = $this->createTestClass(repository);
}
private function createTestClass() {
return new TestedService($repository)
}
}
Стало
Пример стало — сразу под катом. Не хочу повторяться.
Статические значения
В библиотеке из репозитория также есть возможность пробрасывать статические значения через ConstDependencyInjectionParameter. Но это в достаточной мере механическая адаптация уже рассмотренного подхода, поэтому она останется за рамками этой статьи.
DExploN
Очень интересная статья и опыт. Спасибо. Но вот просмотрев основной код, можно выделить 2 основные задачи, которые ими решаются:
1) Короткое создание моков + удаление их в tearDown.
2) Парсинг докблока.
Могу ошибаться, но разве это не сложный велосипед? Для быстрого создания моков есть codeception/stub, а для парсинга докблока — phpDocumentor/ReflectionDocBlock (Но возможно не совсем подходит, конретно под ваш кейс)
Показывая короткий и красивый код в самом тесте, вы, как мне кажется, не учитываете код, который придется писать в докблоке.
Беру теоретичсекий код, который должен работать, и все это заменить:
Разве нет?
sap1986 Автор
Спасибо за указание интересных библиотек.
Сначала отвечу на вопрос по поводу чистого кода теста и чистых phpDoc блоков. Да, вы правильно отметили, что механическое задание объектов для мокирования все равно должно присутствовать в том или ином виде. В нашем случае при указании объектов для мокирования тест получался настолько большим — из-за большего числа зависимостей в старом коде, который необходимо покрывать тестами, — что вариант с раздутыми аннотациями вкупе с возможностями IDE оказался предпочтительней.
В вашем предложении функцию createMock надо явно вызывать и явно указывать ее className, а также явно держать поле для нее: это следствие того, что у нас соглашение не использовать магические ассоциативные массивы, например, из-за плохого хинтинга. Тем не менее, соглашусь, что с случае хорошо написанных классов, которые тестируются, предложенный вариант, возможно, не является оптимальным.
DExploN
Да, данную функцию нужно явно вызывать. Но в вашем случае надо явно прописывать: property className varible. Собственно по количеству символов ваш вариант не короче.
ПротивДа, вы используете класс propertyBag, в котором у вас хранится тот же массив. Не нравится массив? Используйте коллекцию. Да и тайпхинтинга у вас тоже нет. Ведь все создается динамически.
Еще раз повторюсь, ничего не имею против, классная статья, интересная идея и архитектура. Я для себя даже кое что подчерпнул. Но на мой взгляд — излишнее усложнение того, что можно сделать проще. Просто профита не вижу, от написанной сложности.
sap1986 Автор
Еще раз спасибо за фидбек.
У нас в IDE (PhpStorm) Intelli Sens полноценно работает по такому phpDoc: подтягивает типы, подсказывает, как можно мокать по объекту PHPUnit_Framework_MockObject_MockObject, подсказывает, что входит в изначальную сигнатуру объекта. Именно поэтому (полноценно работающий Intelli Sens) такой вариант оказался очень удобным для нас.
Извиняюсь, что не указал это в исходной статье.
oxidmod
Возможно вот это вам поможет.