Здравствуйте, Sinclair, Вы писали:

S>>>Вы предлагаете тестировать этот момент косвенно, через подсчёт реального количества записей, которые вернёт ваш репозиторий, подключенный к моку базы.
S>·>Я предлагаю тестировать не моменты, а бизнес-требования. Не бывает в реальности таких требований как "построить кусочек запроса".
S>Это игра словами.
Это разница между хрупкими тестами, которые тестируют детали реализации и полезными тестами, которые тестируют аспекты поведения.

S>·>Вот только невозможно проверить в _тесте_, что .top(10) действительно добавляется к _любому_ запросу.
S>Я же показал, как именно проверить. Какое место осталось непонятным?
Как проверить что к _любому_ запросу. В твоём _тесте_ проверяется только для одного конкртетного запроса с одним из заданных в тесте criteria.

S>·>Это уже лажа. addLimit это метод некоего QueryBuilder, некий библиотечный метод, который уже имеет свои тесты, включая эти твои отрицательные аргументы. Мы полагаемся, что он работает в соотвествии с контрактом когда им пользуемся.
S>Если AddLimit — библиотечный, то всё прекрасно, и вы правы — тестировать его не надо вовсе.
Из привёдённых снипппетов кода я вижу что именно библотечный и по-другому вряд ли может быть, т.к. код получится совсем другим.

S>·>А зачем этот тест нужен-то? Убедиться, что в двух строчках кода одна из них действительно addLimit? Это и так напрямую явно видно в коде и каждый заметит наличие-отсутствие этого вызова и в PR диффе.
S>Ну вы же почему-то не верите в то, что в двух строчках кода в контроллере берётся именно request.ClientTime и требуете, чтобы это было покрыто тестами

S>·>

S>·>IQueryable<User> buildQuery(UserFilter criteria, int pageSize)
S>·>    => someMagicalCondition(criteria) ? buildOptimizedQuery(criteria) : buildWhere(criteria).addLimit(pageSize);
S>·>

S>·>И твои супер-тесты это не обнаружат.
S>У меня не супер-тесты, а самые примитивные юнит-тесты.
S>·>Т.е. твой тест даст false negative. Или тебе известна методология тестирования кода на простоту и линейность?
S>Основная методология — такая же, как вы привели выше. Каждый заметит наличие-отсутствие этого вызова и в PR диффе, поэтому такой код не пройдёт ревью. От автора потребуют переместить свой buildOptimizedQuery внутрь buildWhere. Если у нас нет под рукой theorem prover, то надёжных автоматических альтернатив нет.
Да банально скобочки забыты вокруг ?: — ошиблись в приоритете операций, должно было быть (someMagicalCondition(criteria) ? buildOptimizedQuery(criteria) : buildWhere(criteria)).addLimit(pageSize). Ведь проблема в том, что мы не описали в виде теста ожидаемое поведение. А тестирование синтаксиса никакой _новой_ информации не даёт, синтаксис и так виден явно в диффе.

S>Впрочем, в вашем подходе всё будет ровно так же — вы же ведь не собираетесь перебирать все мыслимые сочетания значений аргументов в тесте, который проверяет количество записей?
В этом и суть что с т.з. _кода теста_ — проверять наличие addLimit или size()==10 — даёт ровно те же гарантии корректности лимитирования числа записей. Но у меня дополнительно к этому ещё некоторые вещи проверяются и нет завязки на конкретную реализацию, что делает тест менее хрупким.
Ах да. Вы ведь делаете свои тесты красными? Адекватность теста проверить легко — комментируешь "addLimit" и прогоняешь тесты. Если ничего не покраснело, значит надо чинить тесты.

S>·>Мой поинт в том, что ценным тестом будет проверка что addLimit строчка действительно работает в соответствии с бизнес-требованиями — запустить на 11 записях и заассертить, что вернулось только 10 как прописано в FRD — это цель этой строчки, а сама по себе строчка — это средство. И заметь, такой 11/10 тест достаточно сделать для ровно той же комбинации фильтров что и в твоём тесте на наличие addLimit в коде, никакой экспоненциальности которой ты грозишься.
S>Ну это же просто обман. Смотрите за руками: если комбинация фильтров, которая использовалась в тесте на addLimit, попадает под условия someMagicalCondition, то тест сразу сфейлится.
И мой, и твой.

S>А если не попадает — то ващ тест точно так же вернёт 10 записей, и всё.
Верно. Т.е. оба подхода при прочих равных этот аспект проверяют ровно так же. Но твой тест покрывает меньше аспектов.

S>·>т.е. тесты которые так же продолжают работать и ожидаются, что должны быть зелёными без каких-либо изменений после рефакторинга. Такие тесты нужны в любом случае. А твой тест даст false positive и будет просто выкинут. ·>Напишут взамен другой тест, в который закопипастят .contains("rownum < 10") перепутав что rownum zero- или one- based. Отличить на ревью корректность <= от < гораздо сложнее, чем условно save(...11 records...); assert find(10).size() == 10; assert find(5).size() == 5;.
S>Если в проекте завёлся вредитель, то никакими тестами вы его не поймаете. Он просто возьмёт и сломает всё, что угодно. Сейчас вы мне рассказываете, что ловите баги при помощи review кода.
Причём тут вредители? Я очень часто путаю == и != или < и > в коде. Поэтому ещё один способ выразить намерение кода в виде тестового примера с конкретным значением помогает поймать такие опечатки ещё до коммита. Может ты способен в сотне строк обнаружить неверный оператор или забытые строки приоритета, но я на такое не тяну. Поэтому мне надо это дело запустить и просмотреть на результат для конкретного примера.

S>Ну, вот и в нашем случае делается review кода, только код — простой, а за наращивание сложности на ровном месте можно бить человека палкой.
Так ведь ревью делается и для тестов, и для кода. Мне по коду теста сразу видно, что find(10).size() == 10 — правильно. А вот уверено решить что должно быть "rownum > 10", "rownum <= 10" или "rownum < 10" — лично я не смогу, тем более как ревьювер большого PR. Если ты и все члены твоей команды так умеют, то могу только позавидовать.

S>·>Это какой-то тривиальный случай. Это значит не то, что мы магически сделали функцию линейной, а что никаких зависимостей между фильтрами нет в требованиях и никаких 2^N комбинаций просто не нужно, где тут эта ваша грозная data complexity — совершенно неясно.
S>Ну так этой complexity нету как раз потому, что мы её факторизовали. И сделали это благодаря тому, что распилили сложную функцию на набор простых частей.
Какую сложную функцию? Вы просто переписали кривой код на прямой. Тесты тут причём?

S>А вы предлагаете тестировать весь монолит — в рамках него вы не можете полагаться ни на какую независимость. Ну, вот так устроена pull-модель — вам надо построить полный фильтр и выполнить запрос, и никакого способа проверить первую половину запроса отдельно от второй половины запроса не существует.
Я такого не предлагаю.

S>·>В реальности у тебя могут быть хитрые зависимости между параметрами фильтра и вот тут и полезут неявные комбинации экспоненциально всё взрывающие и код несводимый к линейной функции, а будет хитрая if-else-switch лесенка.
S>Нет, по-прежнему будет простая линейная цепочка, только кусочки её будут чуть более длинными.
Да пожалуйста, если получится.

S>·>Гарантию того, что у тебя works as coded. Зато отличить u => u.LastLoginTimeStamp >= criteria.MinLastLoginTimestamp.Value) от u => u.LastLoginTimeStamp <= criteria.MinLastLoginTimestamp.Value) такие тесты не смогут, т.к. пишутся как правило копипастом. На ревью очепятку очень вряд ли кто-то заметит.
S>Если пишут разные люди, то заметят.
Код и тесты обычно пишет один человек. Или вы парное программирование используете?

S>Ну и, опять же, у нас код каждой функции — очень короткий. Там всего-то пара операций.
Если код функции котороткий, то значит у тебя такхи функций овердофига. И в каждой из паре операций можно сделать очепятку.