Re[91]: Что такое Dependency Rejection
От: Sinclair Россия https://github.com/evilguest/
Дата: 07.03.24 14:21
Оценка:
Здравствуйте, ·, Вы писали:

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) такие тесты не смогут, т.к. пишутся как правило копипастом. На ревью очепятку очень вряд ли кто-то заметит.

Если пишут разные люди, то заметят. Ну и, опять же, у нас код каждой функции — очень короткий. Там всего-то пара операций.
Уйдемте отсюда, Румата! У вас слишком богатые погреба.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.