Здравствуйте, ·, Вы писали:
·>Заинжектился в конструктор. Тут код тестов

Автор: ·
Дата: 28.12.23

.
Ну, то есть мои примеры верно отражают степень падения в пропасть.
·>Это то что рассказывает Pauel у него происходит:

·>Ну это то что Pauel предлагает, засовывая Time.now в контроллеры, ставит мне в пику, что я это мокаю: " Во вторых, чуть далее, тоже недавно, вы рассказывали, как моками отрезали Time.now".
Всё правильно он пишет. Вы заменили прямолинейный понятный код, в котором негде сделать ошибку, на код с распределённой ответственностью. Теперь у вас Time.now() расположен максимально далеко от места его использования.

·>У меня другое впечатление.


S>>Вопрос не в том, что есть различные "источники времени", которые производят новые инстанты императивным способом. А просто в том, что "время" — это величина, которая копируется из разных мест, но предсказуемым образом.
S>>И если, скажем, у нас в коде стоит задача "сделать выписку за период от прошлого понедельника до следующей пятницы", то нам категорически противопоказано в коде этого отчёта вызывать lastMonday() и nextFriday(), реализованные в вашем стиле.
·> а в вашем стиле lastMonday(LocalDateTime.now()) и nextFriday(LocalDateTime.now()) можно вызывать, да?!
Можно, но так никто не делает. Потому, что то место, где они нужны — это тоже функция. И в неё по умолчанию не передаётся ссылка на instantSource, а передаются готовые аргументы start и end. Не её дело принимать решения о том, где начало интервала, а где — конец.
Поэтому мы в неё передаём значения. А значения — тоже вычисляются чистой функцией. Чистота означает, что мы не делаем из неё грязных вызовов — поэтому никаких lastMonday(LocalDateTime.now()) и nextFriday(LocalDateTime.now()), а исключительно lastMonday(currentTime) и nextFriday(currentTime)/

·>Ну ясен пень. Это слишком простой вопрос, не стоящий обсуждения. Ну да, мы должны получить некий момент и от него плясать по бизнес-логике. Скажем, тот же TradeDate нужно взять только один раз для данного ордера, в момент его прихода в handler, запомнить его и плясать от него далее, вычисляя всякие там settlement date, days to maturity и т.п.
Ну вы же так не делаете. У вас nextFriday пляшет не от "некоего момента", а от instantSource.now() И вы настаиваете на том, что это — правильный дизайн. А чтобы вы всё-таки написали более-менее рабочую версию с приемлемым дизайном вас нужно долго и нудно загонять в угол. По-моему — всё очевидно.

·>А вопрос об источниках времени — это интересный вопрос, именно его я и обсуждаю. И что один и тот же код, как в недавнем примере, может работать с системным источником RTC в боевой системе, а потом с источником времени из event log во время replay, а ещё и с тестовым источником времени (мок!) во время прогона тестов, чтобы можно было тестировать сценарии "подождём до конца финансового года".
·>Вот это интересное обсуждение.
Ну, пока что ваш код не удовлетворяет предлагаемым вами же требованиям Напомню, что ваш кэш знать не знает ни про какой event log. И возвращает всегда локальное поле, а чтобы его обновить, нужно срабатывание таймера. Таймер ваш никак с instantSource не связан, это прямая дорога к рассогласованию.

·>Я в первую очередь говорю о главном применении моков — отрезать входы-выходы системы (тот самый Time.Now, а ещё сеть, фс, бд, етс), которые в реальности завязываются на что-то тяжело контролируемое или очень медленное.
Ок, давайте откажемся от обсуждения времени — в конце концов, сделать lastFriday достаточно медленной, чтобы наше обсуждение имело смысл, скорее всего не получится.
Правильный способ отрезания входов-выходов — ровно такой, как настаивает коллега Pauel. Потому что в вашем способе вы их никуда не отрезаете — вы их заменяете плохими аналогами. Ващ подход мы, как и все на рынке, применяли в бою. И неоднократно обожглись о то, что мок не мокает настоящее решение; он мокает ожидания разработчика об этом решении.

·>Вот это я не понял. У него в примере тест буквально сравнивал текст sql "select * ...". Как такое поймает это?
Это он просто переводит с рабочего на понятный. Он же пытался вам объяснить, что проверяет структурную эквивалентность. Тут важно понимать, какие компоненты у нас в пайплайне.
Если вы строите текст SQL руками — ну да, тогда будет именно сравнение текстов. Но в наше время так делать неэффективно. Обычно мы порождаем не текст SQL, а его AST. Генерацией текста по AST занимается отдельный компонент (чистая функция), и он покрыт своими тестами — нам их писать не надо.

·>Суть в том, что программист пишет код+тест. В тесте вручную прописывает ожидание =="sellect * ...". Потом тест, ясен пень проходит, потом ревью, который тоже наверняка пройдёт, не каждый ревьювер способен заметить лишнюю букву.
Ну, во-первых, программист же пишет код не в воздухе. Как правило, сначала нужный SQL пишется руками и проверяется интерактивно с тестовой базой, пока не начнёт работать так, как ожидается. Лично я работаю с SQL примерно 30 лет, из них 10 проработал очень плотно. Но и то — сходу не всегда могу написать корректный SQL. Поэтому мы сначала пишем несколько характерных вариантов запроса и прогоняем их на базе (в том числе — используем ровно тот движок, с которым работает прод. Никаких замен боевого постгре на тестовый SQLite или ещё каких-нибудь моков.)
Потом вставляем готовые команды в тест. Потом пишем код.

·>И только потом, когда некий тест пройдёт от логина до отчёта, то оно грохнется, может быть, если этот тест таки использует именно эту ветку кода для тестовых параметров отчёта. Ведь итесты в принципе не могут покрыть все комбинации, их может быть просто экспоненциально дохрена.
У вас — та же проблема, просто вы её предпочитаете не замечать. Вы там запустили тест, а вместо опечатки sellect у вас там вызов функции, которая в inmemory DB работает, а в боевой — нет. SQL вообще состоит из диалектных различий чуть менее, чем полностью.

·>С этим осторожнее. Вот этот твой кеш оказался — с состоянием, внезапно, да ещё и static global.
Да, такие вещи нужно делать аккуратно, вы правы.

S>>Не знаю. А откуда у вас в CalendarLogic берётся instantSource?
·>Через DI/CI.
Ну вот он у вас в тестах — один, в продакшне — другой. Упс.

S>>Ага, то есть у нас где-то есть тот самый код, который выполняет вот это вот
·>Не так. Вот так:

·>CompositionRoot — один на всё приложение, меняется относительно редко, обычно тупой линейный код, и составляет доли процента от объёма всего приложения. Его можно ежедневно глазками каждую строчку просматривать всей командой.
Ну, так это ровно тот же аргумент "против", который вы мне только что рассказывали. Вот вы взяли и заменили ваш код глобально, не разбираясь, где там можно применять кэширование, а где — нельзя.
Потому что ваш кэширующий CalendarLogic полагается на то, что в нём timeSource согласован с timer, а в коде это никак не выражено. Вот вы написали replay, который полагается на возможность замены глобального timeSource на чтение из eventLog, и погоняли его. И так получилось, что данные, на которых вы его гоняли, границу пятницы не пересекали. Откуда вы знаете, что нужно пересечь границу пятницы? Да ниоткуда, у вас в проекте таких "особых моментов" — тыщи и тыщи. Код коверадж вам красит весь ваш CalendarLogic зелёненьким, причин сомневаться в работоспособности нету.

·>И вот такой модуль можно тестировать простыми тестами, подсовывая туда моковый timeSource и ровно этот же модуль будет зваться из "main" метода реального приложения, куда будет запихан SystemClock в качестве timeSource.
Проблема опять только в двух вещах:
1. Слишком много кода в стеке при выполнении тестов
2. Слишком много доверия к тому, что моки адекватно изображают реальные компоненты.

·>И это по всему коду — везде в тысячах мест. Да? И как это тестировать? Как делать replay?
Смотря как мы сохраняем историю для этого replay. Но в целом — код, в котором зашит Time.now(), replay не поддаётся. Это одна из его проблем.
Поэтому у нас там будут вызовы не Time.now(), а какого-нибудь request.ArrivalTime. С самого начала, естественно. Обратите внимание — в каждом вызове время будет своё, и мне достаточно поднять реквест из лога и скормить его ровно тому же методу обработки.
Вам придётся лепить костыли и шаманить, чтобы прикрутить магический instantSource, способный доставать время не из глобального RTC-источника, а из "текущего обрабатываемого запроса".

·>Почему? У нас появляется требование считать nextFriday для платёжки — добавляем новый код для этого, другой код вообще можно не трогать.
Вы же только что поняли, что "другой код" всё же придётся трогать

·>Я обычно использую специальный тестовый шедулер, который запускает установленные таймеры при изменении тестового источника времени. У есть метод типа tickClock(7, DAYS). Код писать лень, но идея, надеюсь, очевидна...
Не, нифига не очевидна. В том-то и дело, что ваш тестовый "источник времени" — это мок, который только и умеет что отдавать заданную вами константу в ответ на now(). Написать его так, чтобы он умел корректно триггерить таймеры можно, но не факт, что вы про это вспомните вовремя

·>Именно. Универсальность — не даёт делать более эффективую реализацию.
Даёт, даёт

S>>И это у вас прекрасно грохнется в продакшне, как только на машине сменят таймзону аккурат около полуночи пятницы.
·>Нет, конечно. Таймеры ставят на instant, т.е. абсолютный момент времени, независящий от зоны.
Вот именно об этом я и говорю. Инстант не наступил, а nextFriday уже должна возвращать новое значение. Упс.

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

·>Через моки же — возвращай что хошь в каком хошь порядке.
Ну так для начала надо захотеть. Вы — не захотели.

·>В сымсле ботва с таймзонами? Ну это у меня уже привычка — писать тесты для дат около всяких dst переходов и с экзотическими таймзонами.
Ботва с таймзонами очень простая: у вас один и тот же instantSource() сначала возвращает "01:00 пятницы", а потом — "17:00 четверга". Просто пользователь перелетел через Атлантику, делов-то.
А у вас уже всё — таймер сработал, nextFriday теперь возвращает следующую неделю.

S>> Отож. И плюс новый набор тестов. При этом старые тесты тоже нужно гонять, удваивая время исполнения. Прикольно, чо. Поднимаем себестоимость на ровном месте.
·>Такие тесты работают порядка миллисекунд. Их можно гонять миллионами и не заметить.
Ну, ок, пусть будут миллионами.

·>Ну смысле если мы из одного юнита А перенесли код в другой юнит Б, то тесты А всё ещё должны проходить. Но они будут идти через А в Б. Это некрасиво. После того как убедились, что А всё ещё работает. Тесты желательно перенести чтобы тестить Б напрямую.
Не перенести, а скопировать. А для полноты ощущений наверное нужно в тестах А заменить Б на мок Б — чтобы это был именно юнит тест, а то уже какая-то интеграция получается.

S>>А как тогда вы защититесь от будущего джуна, который там что-нибудь отвинтит?
·>Не понял, что "что-нибудь"?
Значит возьмёт и поменяет вызов вашего компонента Б на какую-нибудь ерунду.

·>Ну не нравится, не ешь. Об чём спор? Для чего нужен таймер — я тебе написал, для low latency. Без таймера твой вариант у меня тоже элементарно реализуется, правда мне совесть не позволит убить ещё одного котёнка делая static глобальную переменную. Можно даже гибридное решение наворотить, обновляя кеш по таймеру из другого треда, но если вдруг момент timeSource.now() промазывает по кешу, то идти по медленному пути.
Это решение, очевидно, хуже обоих обсуждаемых вариантов. Оно одновременно оборудовано таймером (усложнение зависимостей, хрупкость) и всё ещё иногда идёт по медленному пути.
Так-то и в ФП ваш вариант элементарно реализуется, потому что способ закинуть замыкание в таймер есть более-менее везде.

·>Он удачный в определённых условиях. То что твой подход "удачный" и универсальный всемогутер — твоё большое заблуждние.
Он у вас неудачный в исходных условиях.

·>Это как? Если у нас в SLA написано, что max response time 10ms, а SlowlyCalculatePrevAndNextFridays работает 11ms, то выбора тупо нет, и похрен всем твоя точка зрения.
Ну, ок, я таких SLA не встречал. Это примерно как 100% availability — обеспечить невозможно. Если у вас такой SLA — ну, ок, согласен, это может влиять на дизайн.

S>>В-третьих, там этот "спайк задержки" работает на 1 запрос. Практически невозможно придумать SLA, на которое это повлияет.
·>Открой для себя мир low latency.
С удовольствием. Пришлите ссылку на публичную SLA, в которой есть такие требования, я почитаю.

·>У нас это было серьёзный production incident. Каждый запрос, обработка которого была >10ms — тикет и расследование. Больше таких двух в день — и клиент нам звонит для разборок.
Ну, тогда у нас есть тесты не только функциональные, но и с бенчмарками, и никто не выкатывает в прод код, надеясь, что он быстрый. И ошибки вроде промахов кэша обнаруживаются регулярным способом, а не мамойклянус.

·>Это повлияет только на новый код с платёжкой. Подменить "случайно" по всему приложению так не выйдет. А вот твоя глобальная переменная — happy debugging.
У нас тоже подменить случайно по всему приложению не выйдет — потому что "всё приложение" пользуется функцией nextFriday(x). А кэширование к этой функции приделано только там, где это потребовалось.

·>У меня была цель продемонстрировать не основы объектно-ориентированного анализа, а факт, что можно максимально оптимизировать реализацию nextFriday(), т.к. у нас известно многое внутри самого метода. Навешивая это всё снаружи, приходится делать всё более пессимистично, т.к. инфы тупо меньше.
Нет такого факта, я вам в который раз повторяю. В вашем instantSource нет никакой гарантии, что следующий вызов now() не вернёт момент времени раньше, чем предыдущий.
И по коду, который вы пишете, примерно понятно, как вы будете это делать в продакшн — в частности, вы даже не задумались о том, что CalendarLogic у вас будет декоратором или наследником старого CalendarLogic. Просто взяли и заменили код, на который завязан весь проект. Ну заменили вы "статик глобал" на "глобал синглтон" — что улучшилось-то?

S>>который ФП-программист пишет инстинктивно, т.к. в ФП такая идиоматика.
·>Ага, смешно у Pauel инстинкт с memoize сработал.
Бывает, чо. В реале он бы привинтил к nextFriday memoize, убедился, что тот не помогает, и сел бы разбираться в причинах и искать способ сделать это корректно. Но у него понятная архитектура.

S>>А надо — наоборот: зависимостей — избегать, стейта — избегать.
·>Ага, и ты тут такой: static (DateTime prevFriday, DateTime nextFriday)? _cache А потом хрясь и в догонку nextFriday(LocalDateTime.now())!
Всё правильно — весь стейт торчит на самом верху, и его мало. А глубокие вызовы все pure, и позволяют тестироваться по табличкам безо всяких моков.

S>>Понадобится стейт — всегда можно добавить его по необходимости.
·>А вот попробуй избавиться от этого static и у тебя полезет колбасня.
Не полезет у меня никакой колбасни
Просто будет локальная переменная в замыкании.

S>>Нет у вас никакого широкого простора для оптимизаций. Есть иллюзии того, что timeSource.now() ведёт себя как-то иначе, чем аргумент (x). И, естественно, оптимизации, построенные на этих иллюзиях, будут приводить к фейлам.
·>ЧПФ же. У нас зафиксирован timeSource же.
Ну и что, что ЧПФ? Вы вообще ближе к началу этого поста признались, что вам как таковой instantSource() вовсе не нужен, а нужен ровно один instant, от которого пляшет вся бизнес-логика.
Так и какой смысл подменять параметр типа instant на параметр типа instantSource, который вы хотите позвать ровно один раз в рамках экземпляра бизнес-логики? С какой целью вы это делаете?

·>Так на практике так и бывает. Т.к. в бизнес-логике бизнес-документы, а не алгоритмы 14го века.


·>Сокет ещё утром был подключён к конкретной бирже. Диспечерезацией уже занимается сетевая карта.
Ну и прекрасно — тогда у нас есть глобальная информация о том, с какой биржей мы работаем, внутри messageHandler.

·>Почему неверный? Таймеры тоже срабатывают по timeSource. Проигрывание лога воспроизводит всё поведение системы, но уже без привязки к физическим часам. Ровно это же используется в тестах, когда мы выполняем шаги: "Делаем трейд. Ждём до конца месяца. Получаем стейтмент".
Где у вас это выражено в коде? Как у вас срабатывают таймеры, когда время идёт "назад"?

·>Т.е. делать его нетестируемым. Спасибо, не надо.