Здравствуйте, ·, Вы писали:
S>>Вы предлагаете тестировать этот момент косвенно, через подсчёт реального количества записей, которые вернёт ваш репозиторий, подключенный к моку базы.
·>Я предлагаю тестировать не моменты, а бизнес-требования. Не бывает в реальности таких требований как "построить кусочек запроса".
Это игра словами.
·>Это всё скучные тривиальные детали реализации.
Ну так дьявол-то как раз в деталях.
·>Вот только невозможно проверить в _тесте_, что .top(10) действительно добавляется к _любому_ запросу.
Я же показал, как именно проверить. Какое место осталось непонятным?
·>Это уже лажа. addLimit это метод некоего QueryBuilder, некий библиотечный метод, который уже имеет свои тесты, включая эти твои отрицательные аргументы. Мы полагаемся, что он работает в соотвествии с контрактом когда им пользуемся.
Если AddLimit — библиотечный, то всё прекрасно, и вы правы — тестировать его не надо вовсе.
·>А зачем этот тест нужен-то? Убедиться, что в двух строчках кода одна из них действительно addLimit? Это и так напрямую явно видно в коде и каждый заметит наличие-отсутствие этого вызова и в PR диффе.
Ну вы же почему-то не верите в то, что в двух строчках кода в контроллере берётся именно request.ClientTime и требуете, чтобы это было покрыто тестами
·>·>IQueryable<User> buildQuery(UserFilter criteria, int pageSize)
·> => someMagicalCondition(criteria) ? buildOptimizedQuery(criteria) : buildWhere(criteria).addLimit(pageSize);
·>
·>И твои супер-тесты это не обнаружат.
У меня не супер-тесты, а самые примитивные юнит-тесты.
·>Т.е. твой тест даст false negative. Или тебе известна методология тестирования кода на простоту и линейность?
Основная методология — такая же, как вы привели выше. Каждый заметит наличие-отсутствие этого вызова и в PR диффе, поэтому такой код не пройдёт ревью. От автора потребуют переместить свой buildOptimizedQuery внутрь buildWhere. Если у нас нет под рукой theorem prover, то надёжных автоматических альтернатив нет.
Впрочем, в вашем подходе всё будет ровно так же — вы же ведь не собираетесь перебирать все мыслимые сочетания значений аргументов в тесте, который проверяет количество записей?
·>Мой поинт в том, что ценным тестом будет проверка что addLimit строчка действительно работает в соответствии с бизнес-требованиями — запустить на 11 записях и заассертить, что вернулось только 10 как прописано в FRD — это цель этой строчки, а сама по себе строчка — это средство. И заметь, такой 11/10 тест достаточно сделать для ровно той же комбинации фильтров что и в твоём тесте на наличие addLimit в коде, никакой экспоненциальности которой ты грозишься.
Ну это же просто обман. Смотрите за руками: если комбинация фильтров, которая использовалась в тесте на addLimit, попадает под условия someMagicalCondition, то тест сразу сфейлится.
А если не попадает — то ващ тест точно так же вернёт 10 записей, и всё.
·>Твой тест теоретически не может гарантировать, что некая функция buildQuery возвращает IQueryable с addLimit всегда, для любых входных параметров, такое можно лишь гарантировать через ревью текста самого кода.
·>Цель моего теста: если мы завтра перепишем addLimit на голый sql c "where rownum <= 10" или наоборот — то у нас будет регрессия,
Всё правильно. Вредителей, которые заменяют рабочий код на голый SQL, нужно бить по рукам.
·>т.е. тесты которые так же продолжают работать и ожидаются, что должны быть зелёными без каких-либо изменений после рефакторинга. Такие тесты нужны в любом случае. А твой тест даст false positive и будет просто выкинут. ·>Напишут взамен другой тест, в который закопипастят .contains("rownum < 10") перепутав что rownum zero- или one- based. Отличить на ревью корректность <= от < гораздо сложнее, чем условно save(...11 records...); assert find(10).size() == 10; assert find(5).size() == 5;.
Если в проекте завёлся вредитель, то никакими тестами вы его не поймаете. Он просто возьмёт и сломает всё, что угодно. Сейчас вы мне рассказываете, что ловите баги при помощи review кода.
Ну, вот и в нашем случае делается review кода, только код — простой, а за наращивание сложности на ровном месте можно бить человека палкой.
·>Это какой-то тривиальный случай. Это значит не то, что мы магически сделали функцию линейной, а что никаких зависимостей между фильтрами нет в требованиях и никаких 2^N комбинаций просто не нужно, где тут эта ваша грозная data complexity — совершенно неясно.
Ну так этой complexity нету как раз потому, что мы её факторизовали. И сделали это благодаря тому, что распилили сложную функцию на набор простых частей.
А вы предлагаете тестировать весь монолит — в рамках него вы не можете полагаться ни на какую независимость. Ну, вот так устроена pull-модель — вам надо построить полный фильтр и выполнить запрос, и никакого способа проверить первую половину запроса отдельно от второй половины запроса не существует.
·>В реальности у тебя могут быть хитрые зависимости между параметрами фильтра и вот тут и полезут неявные комбинации экспоненциально всё взрывающие и код несводимый к линейной функции, а будет хитрая if-else-switch лесенка.
Нет, по-прежнему будет простая линейная цепочка, только кусочки её будут чуть более длинными.
·>Гарантию того, что у тебя works as coded. Зато отличить u => u.LastLoginTimeStamp >= criteria.MinLastLoginTimestamp.Value) от u => u.LastLoginTimeStamp <= criteria.MinLastLoginTimestamp.Value) такие тесты не смогут, т.к. пишутся как правило копипастом. На ревью очепятку очень вряд ли кто-то заметит.
Если пишут разные люди, то заметят. Ну и, опять же, у нас код каждой функции — очень короткий. Там всего-то пара операций.