Здравствуйте,
Сегодня тесты выявили такой баг: при сохранении нового клиента, вместо того, чтобы ему выставить некоторую цену доставки груза по умолчанию, эта цена доставки груза выставлялась вообще всем клиентам. Грубо говоря код такой был:
...
if (customer.ID == 0)
{
using (var transactionScope = new TransactionScope())
{
CustomerAccessor.Insert(customer);
CustshipmentsAccessor.SetShipmentPricesForAllCustomers(...); // а надо было CustshipmentsAccessor.SetShipmentPricesForCustomer(...);
transactionScope.Complete();
}
}
...
Т.е. не ту функцию вызывал... К счастью в релиз это не успело уйти.
А обнаружилось это почти случайно, какого-то специального теста на этот случай не было, но так как всем клиентам выставились цены доставки, то слетели пара других тестов. Ну так и обнаружил баг.
Т.е. получается, то, что тест был плохо изолирован (не в отдельной транзакции я его выполнял), сыграло в данном случае мне на руку. А то вообще бы не обнаружил проблему.
Но вопрос в том — как лучше тестировать чтобы такая ошибка обнаружилась. Ведь если для каждой функции писать кучу тестов и код, который проверяет как изменилось состояние БД (не затронулись ли другие строки и таблицы), то с ума можно сойти (я все-таки не маньяк-тестировщик, тесты в основном пишу только на наиболее критичные для бизнеса, либо сложные функции). Можно ли как-то проще?
Здравствуйте, MozgC, Вы писали:
MC>Но вопрос в том — как лучше тестировать чтобы такая ошибка обнаружилась.
Неправильный вопрос.
Правильный такой: Что нужно сделать, чтобы обезопасить себя от таких ошибок.
Тесты — это только один из способов — оборона. Наверное, оборону можно как-то тоже прокачать, но я бы в данной ситуации не сидел в обороне, а наносил бы упреждающие удары. А именно я бы перестал давать методам имена вида ЧтоТоОченьОченьОченьДлинное и ЧтоТоОченьОченьОченьДлинноеДляВсех, чтобы их было не так просто спутать.
Здравствуйте, XopoSHiy, Вы писали:
XSH>А именно я бы перестал давать методам имена вида ЧтоТоОченьОченьОченьДлинное и ЧтоТоОченьОченьОченьДлинноеДляВсех, чтобы их было не так просто спутать.
А предложите для примера свой вариант названия этих двух методов?
Здравствуйте, MozgC, Вы писали:
MC>Здравствуйте, MC>Сегодня тесты выявили такой баг: при сохранении нового клиента, вместо того, чтобы ему выставить некоторую цену доставки груза по умолчанию, эта цена доставки груза выставлялась вообще всем клиентам. Грубо говоря код такой был:
Не уверен по поводу "проще", но в этой функциональности наверняка есть требование "У каждого клиента своя цена доставки", и проверяется, допустим, следующими тестами:
Ничего особо маньячного в этом не вижу, сценарии как сценарии. Полностью защититься от ситуации "а вот на это теста у нас не было" нельзя в принципе. Всегда будут находиться новые ситуации (хотя данный конкретный случай достаточно простой).
MC> CustshipmentsAccessor.SetShipmentPricesForAllCustomers(...); // а надо было CustshipmentsAccessor.SetShipmentPricesForCustomer(...);
MC>Но вопрос в том — как лучше тестировать чтобы такая ошибка обнаружилась. Ведь если для каждой функции писать
Затрудняюсь сказать как тестировать, но знаю как сделать, чтоб такого вообще не появлялось:
НАДО ДАВАТЬ ФУНКЦИЯМ НОРМАЛЬНЫЕ ИМЕНА. ЧИИИИТААААЕЕЕМЫЫЫЫЕЕЕЕЕ! А не блаблабла в пол-экрана шириной.
И не должно быть похожих имён функций, на буковку различающихся. Сильно помогает -- проверено электроникой.
Здравствуйте, fk0, Вы писали:
fk0> НАДО ДАВАТЬ ФУНКЦИЯМ НОРМАЛЬНЫЕ ИМЕНА. ЧИИИИТААААЕЕЕМЫЫЫЫЕЕЕЕЕ! А не блаблабла в пол-экрана шириной. fk0>И не должно быть похожих имён функций, на буковку различающихся. Сильно помогает -- проверено электроникой.
я бы тоже дал такое же имя, можете пример нормального, для данного случая, имени привести?
Здравствуйте, MozgC, Вы писали:
MC>Но вопрос в том — как лучше тестировать чтобы такая ошибка обнаружилась. Ведь если для каждой функции писать кучу тестов и код, который проверяет как изменилось состояние БД (не затронулись ли другие строки и таблицы), то с ума можно сойти (я все-таки не маньяк-тестировщик, тесты в основном пишу только на наиболее критичные для бизнеса, либо сложные функции). Можно ли как-то проще?
В общем случае тестировать то что код не сделал чего-то лишнего (кроме того что сделал то что нужно) слишком накладно.
Но если говорить о коде, работающем с БД, то проверять нужно не в той формулировке, которую я выделил в цитате, а в немного другой: правильно ли произошло изменение состояния БД.
Т.е. нам нужно иметь набор тестовых данных, причем шире чем данные, которые затрагиваются тестируемым кодом. Нужно иметь набор эталонных данных и умение сравнивать состояние БД с эталонными данными. Например, эталонные данные могут быть в формате plain text, тогда нужно уметь выгрузить состояние БД в plain text и сравнить с эталонным набором.
Здесь даже иногда технически проще делать именно интеграционные тесты такого плана, чем юнит-тесты. Юнит тесты потребуют отделения кода BLL от кода, сливающего изменения в БД, и проверок на уровне графов бизнес-сущностей. В то время как интеграционные тесты с БД потребуют лишь утилиты сопоставления состояния БД с набором эталонных данных.
Но так или иначе, эта методика остается все равно относительно неудобной и дорогой для процесса разработки. И принимать решение о необходимости такого тестирования следует руководствуясь оценками рисков, связанных с поставкой неправильно работающего кода, наличием команды (бета)тестеров и т.п.
Или ты поднимаешь вопрос полезности неизолированных тестов?
Здравствуйте, fk0, Вы писали:
fk0> НАДО ДАВАТЬ ФУНКЦИЯМ НОРМАЛЬНЫЕ ИМЕНА. ЧИИИИТААААЕЕЕМЫЫЫЫЕЕЕЕЕ! А не блаблабла в пол-экрана шириной.
Понимаете, в чем дело, щас не старинные экраны в которые по 60 символов влезает, а широкоформатные мониторы, к тому же современные IDE избавляют от необходимости полностью имена классов и методов печатать. Зато если я сам посмотрю на свой код через год — у меня не возникнет вопросов "а что делает этот метод?" — это ясно из названия. То же самое если в проект придет новый человек — ему будет проще понять что делает конкретный класс или метод просто по одному названию.
Здравствуйте, samius, Вы писали:
S>Или ты поднимаешь вопрос полезности неизолированных тестов?
Нет , хотя в данном случае мой плохо изолированный тест безусловно сыграл мне на руку.
В общем пока я сделал пару выводов:
1) Действительно меня подвело имя метода. В изначальном посте я просто по памяти напечатал пример. На самом деле метод назывался CustshipmentsAccessor.UpdateShipmentPrices(int countryID, ...). Если бы имя метода включало упоминание о стране (т.е. что цены выставляются для всех клиентов из страны), то ошибиться было бы сложнее.
2) Такие ситуации протестировать можно, и в принципе несложно (пример привел rlabs), но все-таки это добавит лишних трудозатрат, и при этом все равно не гарантирует того что метод не умудрится изменить чего-то чего не должен Гарантировать можно, только если делать в каждом методе полное сравнение получившегося состояния базы данных с эталонным состоянием, но че-то по-моему это будет геморойный подход (сходу даже не знаю как такое простым способом реализовать для большой базы) и такие тесты будут долго выполняться.
// Есть custService через который можно управлять клиентами
// и только через него.
// У сервиса есть DefaultPriceSet (для клиентов, там все
// ценники по умолчанию для клиентов)
// Метод создающий новый объект Customer требует параметр PriceSet
PriceSet prs = custService.getDefaultPriceSet().clone();
prs.ShipmentPrice = 5.22;
Customer customer = custService.newCustomer(prs);
Вариант 2 (Проверка суперсостояния)
Для БД пишется один метод который проверяет суперсостояние.
Целостность данных в БД трудно проверить полностью констраинтами самой
БД. Поэтому все такие моменты прописываем в отдельный код, который можем
вызывать перед и после тестов.
Однако у такого подхода низнкая применимость. Писать такой тест может
архитектор или ведущий проктировщик, а в таком случае развивать эту
часть кода рядовые программисты либо не могут (нехватает целостного
видения системы), либо не хотят (стремаются лезть в работу вышестоящего).
Т.е. такой подход могут применять небольшие команды 2-4 человека,
исповедующие XP, или хотябы использующие парное программирование и TDD.
Здравствуйте, MozgC, Вы писали:
MC>Здравствуйте, fk0, Вы писали:
fk0>> НАДО ДАВАТЬ ФУНКЦИЯМ НОРМАЛЬНЫЕ ИМЕНА. ЧИИИИТААААЕЕЕМЫЫЫЫЕЕЕЕЕ! А не блаблабла в пол-экрана шириной.
MC>Понимаете, в чем дело, щас не старинные экраны в которые по 60 символов влезает, а широкоформатные мониторы, к тому же
А глаза всё те же, что и в 80-х, как и впрочем мозги. Да и в 80-х в текстовом режиме 132 символа в строке поддерживалось.
fk0>> современные IDE избавляют от необходимости полностью имена классов и методов печатать. Зато если я сам посмотрю на свой код через год — у меня не возникнет вопросов "а что делает этот метод?" — это ясно из названия.
Автодополнение нужно, в основном, где имена функций шириной в пол-экрана -- да... Набрать уж что-нибудь на клавиатуре, при
многолетней тренировке обычно затруднений не вызывает. Что делает метод из названия нифига не ясно. Могут быть многочисленные ньюансы. И что в действительности позволяют современные IDE -- это хотя бы перейти к определению метода и почитать там трёхстрочный комментарий, что он делает в действительности. Не, я конечно тоже против 6-буквенных имён, но есть две крайности...
Здравствуйте, MozgC, Вы писали:
MC>2) Такие ситуации протестировать можно, и в принципе несложно (пример привел rlabs), но все-таки это добавит лишних трудозатрат, и при этом все равно не гарантирует того что метод не умудрится изменить чего-то чего не должен Гарантировать можно, только если делать в каждом методе полное сравнение получившегося состояния базы данных с эталонным состоянием, но че-то по-моему это будет геморойный подход (сходу даже не знаю как такое простым способом реализовать для большой базы) и такие тесты будут долго выполняться.
Такого можно добиться, если каждый Accessor абстрактен настолько, что взамен
реальной базы позволяет виртуальную (in-memory db) использовать. Тогда удаётся
большее число инвариантов проверять за приемлимое время. Да и воссоздать
эталонное состояние внутри теста становится более-менее простым делом.
Кстати, для справки, еще пара примеров, где упрощенный юнит-тест может
обмануть:
1) Самодельные имплементации коллекций. На скорую руку обычно проверяют, что
после add()/remove() элемент присутствует/отсутствует в коллекции и всё.
Правильный подход — как минимум, дополнительно убедиться, что количество
элементов изменилось надлежащим образом и все остальные элементы по-прежнему
в коллекции.
2) Реальный случай с fall-through в switch-конструкции, что в джаве, к сожалению,
допустимо. Информация в торговой системе о трейде приходила с нескольких внешних
систем и switch-case решал с какой комбинацией параметров сохранять этот трейд для
каждого из источников (штук пять кейсов было). В один прекрасный момент случайно
потеряли break — получили по две записи на каждый трейд в базе для одной из систем.
Нашли случайно, когда пользователи возбудились из-за неверной агрегатной суммы по
всем трейдам.
И, да, код-ревью резко снижает вероятность подобных дефектов.
Здравствуйте, MozgC, Вы писали:
MC>Понимаете, в чем дело, щас не старинные экраны в которые по 60 символов влезает, а широкоформатные мониторы, к тому же современные IDE избавляют от необходимости полностью имена классов и методов печатать. Зато если я сам посмотрю на свой код через год — у меня не возникнет вопросов "а что делает этот метод?" — это ясно из названия. То же самое если в проект придет новый человек — ему будет проще понять что делает конкретный класс или метод просто по одному названию.
Даже с большими мониторами, удобнее работать с короткими и лаконичными названиями методов. Об этом и дядя Боб писал, по-моему. Суть скорее не в длине названия, а в его логичности и не спутываемости: если названия отличаются только непного, и это отличие не бросается в глаза, повышается риск ошибки.
Я сам видел код с методами типа ПоискатьЗначениеПоТаблицеИДобавитьЕслиЕгоЕщеНет(), и поддерживать его — совсем не здорово. Другой пример — методы с булевыми параметрами: когда смотришь только на вызов, не всегда ясно отличие между CreateReport( true ) и CreateReport( false ).
Здравствуйте, MozgC, Вы писали:
MC>Здравствуйте, samius, Вы писали:
MC>2) Такие ситуации протестировать можно, и в принципе несложно (пример привел rlabs), но все-таки это добавит лишних трудозатрат, и при этом все равно не гарантирует того что метод не умудрится изменить чего-то чего не должен :( Гарантировать можно, только если делать в каждом методе полное сравнение получившегося состояния базы данных с эталонным состоянием, но че-то по-моему это будет геморойный подход (сходу даже не знаю как такое простым способом реализовать для большой базы) и такие тесты будут долго выполняться.
Можно пойти от обратного. Если есть возможность подменить БД стабом(а без этого как-то странно проводить модульное тестирование), можно отслеживать _затронутые_ вызовом метода объекты БД, и проверять, что других вызовов не было. То есть, какбы, считаем, что метод обновления цен может трогать только таблицы Price и Customer, и если был вызов update любой другой (не важно какой) — тест валится. Хотя, так себе подход.
Здравствуйте, MozgC, Вы писали:
MC>А предложите для примера свой вариант названия этих двух методов?
Что-то я забыл про этот топик
Впрочем можете считать, что я все это время думал над названием
MC> CustshipmentsAccessor.SetShipmentPricesForAllCustomers
Просто потому что после второй точки придется явно выбрать ForAll или не фор олл.
А вообще, конечно, все зависит от того, что там ещё в этом CustshipmentAccessor живет. Может быть есть варианты и более натуральные.
Здравствуйте, rlabs, Вы писали:
R>Здравствуйте, MozgC, Вы писали:
MC>>Здравствуйте, samius, Вы писали:
MC>>2) Такие ситуации протестировать можно, и в принципе несложно (пример привел rlabs), но все-таки это добавит лишних трудозатрат, и при этом все равно не гарантирует того что метод не умудрится изменить чего-то чего не должен Гарантировать можно, только если делать в каждом методе полное сравнение получившегося состояния базы данных с эталонным состоянием, но че-то по-моему это будет геморойный подход (сходу даже не знаю как такое простым способом реализовать для большой базы) и такие тесты будут долго выполняться.
R>Можно пойти от обратного. Если есть возможность подменить БД стабом(а без этого как-то странно проводить модульное тестирование), можно отслеживать _затронутые_ вызовом метода объекты БД, и проверять, что других вызовов не было. То есть, какбы, считаем, что метод обновления цен может трогать только таблицы Price и Customer, и если был вызов update любой другой (не важно какой) — тест валится. Хотя, так себе подход.
А я бы еще добавил, что юнит тестирование, почти всегда не совместимо с проверкой чего-то, что не написано вами (вашей командой).
В данном случае, тестировать надо то, что был вызван именно тот метод, который предполагалось использовать.
Проверка того, как изменились данные в базе при вызове метода — это уже задача интеграционных тестов.
Если брать пример топик стартера, то тут тест был бы примерно таким:
Если писать в режиме TDD, то чтобы допустить такой баг, нужно ошибиться и в тесте и в коде одновременно.
Все что остается, это проверить (уже в наборе интеграционных тестов), что SetShipmentPricesForAllCustomers действительно обновляет все записи, а SetShipmentPricesForCustomer только для конкретного покупателя.
Ну и потом провести рефакторинг, чтобы не путать имена.
Здравствуйте, Gmoorick, Вы писали:
G>В данном случае, тестировать надо то, что был вызван именно тот метод, который предполагалось использовать.
Я боюсь, что если писать тест после кода, то так же просто возьмешь и в тесте тоже выберешь не тот метод (потому что перед этим сначала поглядишь на тестируемый метод и увидишь там ...FoAllCustomers() — и поэтому автоматом напишешь Mock.VerifyWasCalled(()=>MockedCustshipmentsAccessor.SetShipmentPricesForAllCustomers(...));
Но наверное это уже переходит в разряд, что если мозг не напрягать, то что угодно можно сломать.
Здравствуйте, MozgC, Вы писали:
MC>Здравствуйте, Gmoorick, Вы писали:
G>>В данном случае, тестировать надо то, что был вызван именно тот метод, который предполагалось использовать.
MC>Я боюсь, что если писать тест после кода, то так же просто возьмешь и в тесте тоже выберешь не тот метод (потому что перед этим сначала поглядишь на тестируемый метод и увидишь там ...FoAllCustomers() — и поэтому автоматом напишешь Mock.VerifyWasCalled(()=>MockedCustshipmentsAccessor.SetShipmentPricesForAllCustomers(...));
MC>Но наверное это уже переходит в разряд, что если мозг не напрягать, то что угодно можно сломать.
именно поэтому я написал про TDD, юнит-тест после кода (если это не дебаггинг)- это имхо время на ветер.
, хотя в данном случае мой плохо изолированный тест безусловно сыграл мне на руку.
Гарантировать можно, только если делать в каждом методе полное сравнение получившегося состояния базы данных с эталонным состоянием, но че-то по-моему это будет геморойный подход (сходу даже не знаю как такое простым способом реализовать для большой базы) и такие тесты будут долго выполняться.
