Комментарии 35
- Замокать
MessageQueue.Receive()
- Вернуть из замоканного метода сообщение, которое вызовет сработку нового флага.
- Вызвать
MessageProcessorService.Start()
- Проверить, что обработка была выполнена правильно (судя по всему, ProcessMessage имеет побочные эффекты)
- ?????
- Вы великолепны.
И да, TDD заключается в том, что сначала пишется тест, а потом под него код. Вы же пытаетесь натянуть сову на глобус, написав сначала ущербный код, а потом пытаясь его протестировать, запрещая при этом его изменять. Если бы ваш сервис принимал делегат для `MessageQueue.Receive` и существующий экземпляр токена отмены, проблем бы с тестированием не было бы никаких, независимо от потоковой модели вашего сервиса.
Если же получится тест, который прост в поддержке (т.е. легко найти в нем проблему, если он упал), легок в обращении (т.е. если он перестал быть актуальным, его можно удалить, а вместо него написать другой), и при этом на его написание тратится 10 минут, то это хороший тест. И это ближе к TDD.
Относительно internal. Да, какая-то логика становится видна для тестов. Но это не страшно, т.к. тесты легкие и понятные. Если начали падать, то понятно почему. Если стали неактуальны — удаляем и пишем другие. Кроме того, internal — это еще не public. Internal не является хинтом, что этот метод должен быть использован везде, где можно. Кроме того, обычно в команде на этапе Code Review пресекаются неправльные использования. Да, это не идеальный TDD, но и громоздить код в настройках юнит-тестов публичных методов, и заниматься рефакторингом, по сути, ради рефакторинга, нет необходимости.
Если тест для публичного метода, написан через рефакторинг, сделаный под этот тест, то это плохой тест.
Нет, неправда. В этом случае плох отнюдь не тест, а код, который мы тестируем. Модульные тесты описывают частные случаи использования публичного контракта объекта в изолированной среде. Если нужный нам частный случай не может быть протестирован, значит, объект не решает поставленную задачу и тестируемый код надо менять. TDD — оно именно про разработку через тесты, а не про "давайте напишем тесты к старому коричневому коду". Не нужно путать термины :)
А я рассказываю о более реальной ситуации, когда есть говно-код, и все равно надо с ним работать и его тестировать.
Вы Физерса не читали?
К тому, что описанное в вашем посте точно попадает под описания из книги.
Задача: внедрить новый флаг бизнесс-логики в обработку сообщений.
- Chapter 8: How Do I Add a Feature?
Тестируем ProcessMessage
Chapter 10: I Can’t Run This Method in a Test Harness
[...]
"Fortunately, there is a very direct answer for this question: If we need to test a private method, we should make it public. If making it public bothers us, in most cases, it means that our class is doing too much and we ought to fix it. [...] The real reason we can’t test the [...] class well is that it has too many responsibilities. Ideally, it would be great to break it down into smaller classes using the techniques described in Chapter 20..."
Chapter 20: This Class Is Too Big and I Don’t Want It to Get Any Bigger
Я, собственно, некоторое время не мог понять, почему мне ситуация из поста кажется настолько тривиальной — вот именно поэтому: она описана и обсуждена больше десяти лет назад.
Почему есть метод Start() но нет метода Stop()? Зачем странно используемый lockObject?
Если всё это исправить(ну и многое другое) и сделать аккуратно и красиво то после 3) вызывается синхронный Stop() после которого проверяются все побочные эффекты
Идеальный вариант (если позволяет время и бюджет) сначала переписать этот код и покрыть тестами, затем добавить флаг и покрыть флаг тестами.
Я думаю что у каждого есть свой говнокод, который без костылей не протестировать. Применять ли тут паттерн «паблик морозов» я не знаю. Стоит ли рассказывать как тестировать говнокод с помощью костылей я тоже не знаю)
Дальше смысла писать нет. Статья про тестирование нетестируемого кода, просто за подобный код надо руки отрывать, а не предлагать мокать то, к чему нет доступа.
Всё переписывать с нуля? А если бизнес не согласен это оплачивать, то в своё свободное время?
Я не специалист в c#, прошу меня запардонить, но разве в c# нельзя заменить системный класс своим на этапе компиляции? Т.е. сказать "то, что вам ранее было известно как System.Random, это теперь MySuperRandom"?
Ну ладно, пусть нельзя на этапе компиляции — можно ж наверное тогда перегрузить системную assembly своей? Что-то типа LD_PRELOAD, только для .net? Заменить все системные классы пустыми Proxy-подобными обертками, у которых нет своей логики, кроме как перенаправлять вызовы либо на оригинальные реализации, либо, если стоит флажок, дергать класс-делегат.
Если даже нельзя, остается хардкор — можно ведь пропатчить MSIL для целевого метода на ходу? Типа вот этого сплайсера? Или вот этой библиотеки (которая, как мне кажется, внутри делает то же самое)?
Да, естественно, это unsafe, но это же тест, а не продовый код. Если тест упадет, никто не умрет, тесты для того и предназначены.
Microsoft Fakes. Но про TDD после этого можно забыть.
Потому что когда у вас есть возможность влезть куда угодно и подменить что угодно, вы совсем иначе думаете о структуре кода, нежели когда у вас такой возможности нет. Памятуя о максиме "тестируемый дизайн — хороший дизайн", если вы уберете давление со стороны тестируемости, вы понизите качество кода (ну или по крайней мере, драйвер в сторону качества).
На вскидку, я бы не стал делать ProcessMessage pubiс. Судя по InitializeReceiveLoop он не самодостаточен.
Как вы объясните зачем нужны такие большие изменения ради одного флага?
Давайте по шагам.
Ваш
MessageProcessorService
нарушает SRP, хоть и слегка — в нем одновременно есть логика работы с очередью и логика обработки сообщений.
Ваш
MessageProcessorService
неудобен для тестирования, потому что в нем есть асинхронный код, который тестировать всегда неудобно.
Решение второй проблемы — сделать ProcessMessage
видимым для тестов, не важно, как именно (зависит от доступного вам инструментария), и тестировать только его.
Решение обеих проблем — вынести обработку сообщений в отдельный класс (надо показывать, как?) и тестировать его стандартным способом без каких-либо проблем. В качестве бонуса гарантируем себе отсутствие лишней связности между очередью и обработкой сообщения.
Задача: внедрить новый флаг бизнесс-логики в обработку сообщений. Этот флаг имеет нетривиальную логику. Как будете внедрять и тестировать флаг?
Что значит "внедрить новый флаг"? Это неверная постановка задачи, особенно в терминах кода, который вы показываете. Нужно добавить новую ветку выполнения внутрь ProcessMessage
на основании чего-то в полученном сообщении? Или что?
(поэтому, кстати, и ваше "я внёс бы несколько изменений" выглядит бессмысленным — мне, например, совершенно непонятно, что такое "метод GetFlagValue
для логики флага", и зачем там вообще метод)
кто бы ни вносил изменения, я ожидал бы, что исправлен будет только один метод, а юнит-тест имел бы отношение только к изменяемой логике.
Это ожидание неверно. "Только один метод" может быть исправлен только при соблюдении SRP (у вас это не так), и даже тогда не обязательно. А юнит-тест будет иметь отношение только к изменяемой логике в том случае, если есть юнит-тесты на логику до изменения. У вас это так?
если метод можно использовать только внутри основного класса, где он может быть вызван из любого другого метода, то делайте его protected internal
.
Нет никаких оснований ни для protected
, ни для internal
.
PS Ни тег TDD, ни "TDD — это долго" тут ни при чем: в посте нет ни следа TDD.
Со в торой частью согласен. Надо поработать над качеством изложения, т.к. много осталось «за кадром».
А самое простое решение поставленной проблемы имеет классическое описание: вместо мутной логики в приватном методе делаем публичный класс, в который помещаем ту же самую логику и пишем столько тестов, сколько захотим. Приватный метод превращается в банальное обращение к этому классу и уже не требует собственных тестов.
Реальная же проблема, которая наблюдается в вашем примере, и присутствует в куда более жёстких проявлениях во всякий древних проектах — это макаронный код, где вперемешку логика и работа с внешним состоянием и внешними сервисами. Вот там да, на первый полезный тест можно пару недель убить…
Для того, чтобы проиллюстрировать проблему, придумал пример.
Какую проблему вы иллюстрируете?
Но сейчас никто не станет его переписывать (или рефакторить), т.к. это приведет к большим затратам времени на правку и тестирование того, что и так работает.
… и тем не менее, именно этот вариант решения является основным (и, более того, не предполагает "больших затрат времени" ни на правку, ни на тестирование).
Так какую же проблему вы рассматриваете?
Когда тестирование через public-метод начинает вонять (пример)