Pull to refresh

Comments 56

Поругайте что ли. Не уж-то нулевая актуальность?
Возможно, из-за того, что статья, специфичная для C#, в блоге TDD, и поэтому ее читает меньшее количество людей, а те, кто читают, понимают хуже? Ну понятно, что разработчики, использующие другие языки, могут поднапрячься и разобраться, что тут написано и как это применить к своей системе (и можно ли применить), но текст в этом никак не помогает.
К сожелению лучшего блога для этого я не знаю: тестирование — нет, разработка — нет, C# — нет. А этот блог объединяет TDD и автоматическое тестирование.
Плюсанул статью первый вроде, но решил, что комментарий типа «Спасибо, познавательно, когда и если буду писать многопоточные приложения на C#, то перечитаю, а пока пошел писать тесты на PHPUnit» будет обидным.
Совершенно не понятно какая проблема решается. Да и вообще о чём пост. Без внятного введения трудно как-то реагировать.
Упращение построения автоматических тестов для ассинхронной системы.
Так а что тестируете?

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

Повторюсь: данное замечание имеет право на существование, но вопрос слишком сложный что бы о нем говорить так категорично.
«Работают только в режиме отладки (при определенном макросе DEBUG). Мы не должны задумываться, что дополнительный код повлияет на работу системы у конечного пользователя.»
Это (как и любой другой условный код) означает, что ваши тесты не тестируют систему, как она будет у пользователя. А это, в свою очередь, означает, что их актуальность при рефакторинге уменьшается.

Хамбл и Фарли пишут в Continuous Delivery, что автоматическое тестирование многопоточных и асинхронных систем следует сводить к синхронным тестам (Humble and Farley, Continuous Delivery, 2010, стр. 170).
Оно так и получается. Строчки Breakpoint.Define("...") синхронизируют проверяемые фрагменты до такой степени, что они начинают работать почти синхронно (почти верно: в один момент времени, один работающий поток). На мой взгляд в таком контексте — это не противоречит «Хамблу и Фарли».

По поводу DEBUG. Справедливо. Но на мой взгляд предназначение DEBUG режима — отладка системы. Одна из ролей автоматических тестов — это отладка.

Вообще данный вопрос очень сложен. Я дважды участвовал в дискуссии по нему с совершенно разными людьми. И не разу это не привело к маломальскому консенсусу. Помоему, это холивар.
«На мой взгляд в таком контексте — это не противоречит «Хамблу и Фарли».»
Противоречит, потому что вы все равно имеете больше одного потока. На уровне юнит-теста надо тестировать код в каждом потоке отдельно, а на уровне компонентного можно применять и ваш подход, в принципе — с оговоркой про DEBUG.
Допустим, пара потоков пишет объекты в список, а пара считывает и обрабатывает их.

Можно разбить логику тестирования на тесты:
1. Если была запись, то проходит и чтение.
2. Если записи не было, то при чтении метод залипает (например).
3. Возможность записывать двумя потоками.
4. Возможность читать двумя потоками.
5. Возможность читать и писать одновременно.

Первые два теста возможно сделать абсолютно синхронными, три последних нет (в асинхронности весь смысл).

Можно поставить какие-нибудь объекты синхронизации не глядя в код, но где гарантия что они будут делать реально то (и только то) что действительно нужно.
Вы не можете достоверно протестировать поведение 3-5, потому что ни один тест-процессор не гарантирует вам, что потоки действительно будут работать «одновременно», а не «последовательно».

Поэтому для вашего конкретного примера надо использовать какую-нибудь существующую конкурентую коллекцию, которых (например) в .net больше одной, и радоваться, что всю работу сделали за вас. Вы же не тестируете string.Format? Вот и это тестировать не надо.

(после чего коллекция абстрагируется до нужного интерфейса, и в тех местах, где надо проверить, что код-потребитель корректно реагирует на многопоточные события, такие события имитируются)
В том то и дело, что могу. Описанный в статье инструмент преобразует понятие «одновременно» в «последовательно». С тем порядком который необходим, для подтверждения того что система реально блокирует возможность обратиться к разделяемому ресурсу одновременно.

По поводу «существующую конкурентую коллекцию». Любая абстракция дырявая по определению, то есть в любой момент можно столкнуться с задачей которая не решается существующими средствами, или существуют особенности делающие их не применимыми в конкретном случае. Например, какая-нибудь хитрая приоретезация.
«В том то и дело, что могу. Описанный в статье инструмент преобразует понятие «одновременно» в «последовательно». С тем порядком который необходим, для подтверждения того что система реально блокирует возможность обратиться к разделяемому ресурсу одновременно.»
То ли я вас не понимаю, то ли вы меня.

Вот есть конкурентная очередь, у нее есть методы Enqueue и Dequeue. Весь смысл этой коллекции в том, чтобы гарантировать атомарность этих методов. Коллекция предоставлена фреймворком, т.е. влезть внутрь нее вы не можете.

Чтобы протестировать ее поведение в условиях коллизии, нужно, чтобы обращения к q.Enqueue и q.Dequeue (или второму q.Enqueue) произошли «одновременно». В любом другом случае коллизии заведомо не будет.

И как это сделать вашим механизмом?

«Любая абстракция дырявая по определению, то есть в любой момент можно столкнуться с задачей которая не решается существующими средствами, или существуют особенности делающие их не применимыми в конкретном случае. Например, какая-нибудь хитрая приоретезация. „
Не надо поминать Спольски не по делу. Задача, которая не решается существующими средствами — это не дырявая абстракция, это просто другая задача (и, как следствие, неприменимая абстракция).

Опыт показывает, лучше всего сводить любую сложную многопоточную задачу к набору тривиальных (и синхронно тестируемых) операций поверх стандартных примитивов.
Зачем мне тестировать конкурентная очередь из внешнего фреймворка? Ее авторы мне за это не заплатят.

Если же необходимо сделать нечто особенное, что она не учитывает (например хитрым способом соединить две очереди), то в своей прокладке кода я смогу поставить Breakpoint-ы и проверить это взаимодействие.
«Зачем мне тестировать конкурентная очередь из внешнего фреймворка?»
Затем, что любые абстрации текут.

Об этом и речь, что в среднем лучше всего свести задачу до того уровня, когда вся мультитредность лежит на фреймворке, а весь код вокруг него — синхронный. И тестировать синхронно.

Потому что, повторюсь, никакой брейкпойнт не симулирует вам реальную конкуренцию.
На самом деле, обсуждать это на примерах «идеальных задач» — бессмысленно.

Давайте отталкивать от реальной системы, которую надо тестировать, и думать, как ее тестировать. Тогда, может быть, станет понятно, зачем делать все это… странное.
Последняя решаемая с помощью этого инструмената задача — был диспетчер отложенных задач для wpf-приложения.

Пример сценария:
1. Пользователь из списка выбирал элемент. Реакцие на это — установка пакета задач в диспетчер с задержкой например 0.5 сек
2. Если пользователь в течении этого времени не делал нового выбора, то пакет начинал исполняться. Нет — новый заменял старый с новой задержкой 0.5 сек
3. Пакет состоял из обращения к БД и заполнения соответствующего контейнера данных, на который смотрел DataGrid. Первая задача выполнялась на ассинхронном потоке (для обеспечения отзывчивости интерфейса), вторая на потоке окна. По потокам раскладывает диспетчер.
4. В случае ошибки — уведомление и в зависимости от типа завершение или нет приложения. Пакет безусловно прерывается.

Пакеты и задачи исполняются последовательно относительно друг-друга. Выполнение пакета не блокирует добавление нового. Приоритизация пакетов по (время установки+задержка). Идентификация пакета по объекту маркера, определяемого при создании пакета.
Лично для меня эта задача слишком сложна и для реализации в один прием, и для тестирования. Поэтому начнем с кубиков. Очевидно, что задачу можно подробить на более мелкие (с тривиальным взаимодействием), и реализовывать/тестировать их.

Пойдем с конца: начиная с п.3 (включая примечания по последовательности и блокировкам) мы видим обычную очередь выполнения. Ее можно отделить. Соответственно, перед этим мы видим компонент, который получает в себя задачу, и выжидает t (если за t пришла новая задача, старая выкидывается, начинается новое ожидание, если не пришла, задача встает в очередь выполнения).

Согласны?
Давайте попробуем.

Предложение — сценарий есть. Может заставим его работать вместе? Заодно лишний раз польем воды на мельницу TDD, создав реальный механизм.

Если да, то с вас тест.
Для того, чтобы тестировать _сценарий_, нужен тест-кейс, а для него — человек с мозгами тестировщика. Я по этому и перешел к компонентам.

Так вот, если мы согласились, что мы можем разбить сценарий на эти два компонента, то давайте тестировать их по отдельности. Очередь выполнения пока выкидываем (это стандартный Customer/Producer, по большому счету, и реализуется вокруг стандартной же блокирующей очереди), и смотрим на первый компонент, как менее тривиальный.

Поведенческий тест 1: добавить задачу, выждать t, удостовериться, что задача пошла в очередь выполнения
Поведенческий тест 2: добавить задачу 1, выждать t+1, добавить задачу 2, удостовериться, что в очередь пошли задачи 1, а потом 2.
Поведенческий тест 3: добавить задачу 1, выждать t/2, добавить задачу 2, удостовериться, что в очередь пошла только задача 2.

Пограничные тесты:
(1) «одновременно» добавить две задачи, удостовериться, что в очередь встала одна (любая, потому что поведение компонента не определено) из них
(2) добавить задачу 1, выждать ровно t, добавить задачу 2, удостовериться, что в очередь пошла «нужная» (в зависимости от спецификации компонента, у вас не уточнено) задача

Согласны?
		[TestMethod]
		public void ExecTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time = null;

				disp.Add(TimeSpan.FromSeconds(1), () => time = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(2));

				Assert.IsTrue(time - now > TimeSpan.FromSeconds(1));
			}
		}

		[TestMethod]
		public void ExecTwoTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time0 = null;
				DateTime? time1 = null;

				disp.Add(TimeSpan.FromSeconds(1), () => time0 = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(2));

				disp.Add(TimeSpan.FromSeconds(1), () => time1 = DateTime.Now);

				Assert.IsTrue(time0 - now > TimeSpan.FromSeconds(1));
				Assert.IsTrue(time1 - time0 > TimeSpan.FromSeconds(1));
			}
		}

		[TestMethod]
		public void ReplaceTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time0 = null;
				DateTime? time1 = null;

				disp.Add(TimeSpan.FromSeconds(2), () => time0 = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(1));

				disp.Add(TimeSpan.FromSeconds(2), () => time1 = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(3));

				Assert.IsNull(time0);
				Assert.IsTrue(time1 - now > TimeSpan.FromSeconds(3));
			}
		}

		[TestMethod]
		public void AddSomeTimeTwoTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time0 = null;
				DateTime? time1 = null;

				//как-то и где-то в отдельных потоках
				disp.Add(TimeSpan.FromSeconds(1), () => time0 = DateTime.Now);
				disp.Add(TimeSpan.FromSeconds(1), () => time1 = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(2));

				//только один не null
				Assert.IsTrue((time0 != null) != (time1 != null));
			}
		}

		[TestMethod]
		public void AddSomeTimeTwoTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time0 = null;
				DateTime? time1 = null;

				//как-то и где-то в отдельных потоках
				disp.Add(TimeSpan.FromSeconds(1), () => time0 = DateTime.Now);
				disp.Add(TimeSpan.FromSeconds(1), () => time1 = DateTime.Now);

				Thread.Sleep(TimeSpan.FromSeconds(2));

				//только один не null
				Assert.IsTrue((time0 != null) != (time1 != null));
			}
		}

		[TestMethod]
		public void AddNewFromEndFirstTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime now = DateTime.Now;
				DateTime? time0 = null;
				DateTime? time1 = null;

				disp.Add(TimeSpan.FromSeconds(1), () => time0 = DateTime.Now);

				//где-то точно (точно во время схватывания первой задачи)
				Thread.Sleep(TimeSpan.FromSeconds(1));

				disp.Add(TimeSpan.FromSeconds(1), () => time1 = DateTime.Now);
				
				//только один не null
				Assert.IsTrue((time0 != null) != (time1 != null));
			}
		}


Будем считать, что вы написали это (если нет возражений).

Мне кажется в каждом из тестов есть явные проблеммы — синхронизация по sleep
Возражения, конечно, есть.

Во-первых, я не вижу, согласились ли вы с предложенными сценариями для тестирования, а во-вторых, совершенно не понимаю, как написанный вами тест с ними соотносится.
Я попытался выразить предложенный вами псевдокод в коде. Не нравится — предложите свой вариант тестов в коде. А то остается слишком много места для разночтений и волшебства.

По поводу согласия — опять нужен код. А то вы меня потом как котенка будете тыкать носом в то о чем я не говорил.
const TimeSpan t =…

[TestMethod]
public void SingleTask_ShouldQueueAfterTimeout()
{
var timeService = new TimeServiceStub();
var queue = new QueueMock();
var target = new DelayedExecution(timeService, queue);
var task = new TaskStub();
target.Add(task);
timeService.Wait(t);
queue.AssertEnqueuedOnlyOne(task);
}

[TestMethod]
public void MultipleTasks_ShouldQueueInSequence()
{
var timeService = new TimeServiceStub();
var queue = new QueueMock();
var target = new DelayedExecution(timeService, queue);
var task1 = new TaskStub();
var task2 = new TaskStub();
target.Add(task1);
timeService.Wait(t+1);
target.Add(task2);
timeService.Wait(t);
queue.AssertEnqueuedExactSequence(new []{task1, task2});
}

[TestMethod]
public void MultipleTasks_ShouldQueueInSequence()
{
var timeService = new TimeServiceStub();
var queue = new QueueMock();
var target = new DelayedExecution(timeService, queue);
var task1 = new TaskStub();
var task2 = new TaskStub();
target.Add(task1);
timeService.Wait(t/2);
target.Add(task2);
timeService.Wait(t);
queue.AssertEnqueuedOnlyOne(task2);
}

Это псевдореализация поведенческих тестов. Она вас устраивает?
Это
Пойдет. Хотя мои выглядят лучше :) (но все равно +1)

Я даже понял как вы будете реализовывать эту логику.

Если не возражаете, давайте теперь реализуем timeService. Помоему, он наиболее интересен. Так же тестами.
Если без учета производительности, то тривиально:

public void Wait(TimeSpan interval)
{
Thread.Sleep(interval);
}

Эта штука, прямо скажем, в сервис-то вынесена только по заветам Хамбла, который абстрагирует любые операции со временем.
Я почему-то подумал, что задача перетягивания task-ов дело хитрого Wait-а используемого в том числе и в DelayedExecution.Run (имя от балды).

Как тогда?
Вся работа происходит внутри DelayedExecution.Add, который мы, собственно, и тестируем.

У нас ведь весь разговор о тестах, а не о реализации?
Слишком мало пространства осталось для творчества. Если не возражаете давайте продолжим здесь
Остается только target.Add(task2). Давайте тогда его посмотрим. Он должен сохранить где-то на себе элемент и через заданное время перетянуть его на очередь. Даже без замены задач — это не так просто сделать. Мы оказываемся в исходной точке.
Я бы конечно начал с чего-то более общего. Но если хотите п.3 пусть будет п.3
Ну тесты без реализации это нонсенс. В TDD они движущая сила построения архитектуры. Давайте все таки попробуем реализовать его, хотя бы так, что бы мы оба одинаково поняли как это сделать.

А то по моему мы пришли к исходному состоянию не решенности задачи.
Просто сначала надо понять, как мы тестируем, а потом уже писать то, что мы тестируем. Просто обратите внимание, что тест прекрасно обошелся без вашего BreakpointManager.

class DelayedExecution {


const TimeSpan Delay; //на самом деле, его надо было передавать снаружи, как и t, но я про это не подумал еще в тесте

private readonly _lock = new object();
private CancellationTokenSource _currentTaskCancellation;

public void Add(ITask task)
lock (_lock) {
if (_currentTaskCancellation != null)
_currentTaskCancellation.Cancel();

_currentTaskCancellation = new CancellationTokenSource();
var t = new Task(() => worker(task, _currentTaskCancellation.Token), _currentTaskCancellation.Token);
t.Start();
}
}

private void Worker(ITask task, CancellationToken cancellationToken)
{
var wait = new Task(() => _timeService.Wait(Delay), cancellationToken);
wait.Start();
wait.Wait(cancellationToken);
//а здесь выполняем работу по таску, сюда мы попадем только если ничего не было отменено
}
}

Приблизительно так, детали надо уточнять по документации на cancellable tasks.

Но понимаете ли, речь не об этом; речь о том, как протестировать поведение снаружи компонента. Мы ведь в блоге TDD, а не .net/development?
«Просто обратите внимание, что тест прекрасно обошелся без вашего BreakpointManager»

При этом не протестировав ничего. Была у нас задача накопления task-ов, она и осталась. Была задача по приоритезации, она так же не куда не делась. Все чего мы добились тремя тестами — это перешли с уровня модуля «класс», к уровню «публичный метод» (остальные это часть черного ящика).

А затем «на зубах»(с) написали решение. В правильности которого ни какой уверенности нет, потому что оно без тестов. Страх перед рефакторингом на том же уровне. Дальнейшее эволюционное развитие представляется затруднительным.
«При этом не протестировав ничего»
Почему? Мы выделили два компонента, для первого полностью описали поведение, и протестировали его. Нет?

«Была у нас задача накопления task-ов, она и осталась.»
Это второй компонент, помните? Мы его еще не начинали рассматривать.

«Была задача по приоритезации, она так же не куда не делась»
Если честно, этого места в исходной формулировке я не понял. Но в любом случае, это тоже задача второго компонента (очереди).

«В правильности которого ни какой уверенности нет, потому что оно без тестов.»
Как это без тестов? А что за пять тестов выше, с которыми вы согласились?
Задача накопления решается вами здесь:

var t = new Task(() => worker(task, _currentTaskCancellation.Token), _currentTaskCancellation.Token);
t.Start();
<source>

Приоретизация действительно не решена:

<source lang="cs">
[TestMethod]
public void MultipleTasks_ShouldQueueInSequence()
{
var timeService = new TimeServiceStub();
var queue = new QueueMock();
var target = new DelayedExecution(timeService, queue);
var task1 = new TaskStub();
var task2 = new TaskStub();
target.Add(task1);
timeService.Wait(t/2);
target.Add(task2);
timeService.Wait(t);
queue.AssertEnqueuedOnlyOne(task2);
}


«Как это без тестов? А что за пять тестов выше, с которыми вы согласились?»
А вот это про котят :) Не пять, а три (в коде). При представленной вами реализации сомневаюсь что они срабатывают (код ни как не учитывает третьего теста; да и первые два сомнительно — ведь сценарий вы ограничили перекладыванием задач в очередь, чего не сделали).

Ну и удар ниже пояса:
"… в среднем лучше всего свести задачу до того уровня, когда вся мультитредность лежит на фреймворке, а весь код вокруг него — синхронный. И тестировать синхронно."
«Задача накопления решается вами здесь:»
Это не накопление, это постановка конкретного таска. Если вы обратите внимание, предыдущий таск всегда отменяется.

«Приоретизация действительно не решена:»
… и не должна быть.

«При представленной вами реализации сомневаюсь что они срабатывают (код ни как не учитывает третьего теста; да и первые два сомнительно — ведь сценарий вы ограничили перекладыванием задач в очередь, чего не сделали). „
Это, признаю, моя ошибка. Но на то и TDD, чтобы дать мне red bar. Для превращения его в green bar достаточно заменить “//а здесь выполняем работу по таску, сюда мы попадем только если ничего не было отменено» на _queue.Enqueue(task).

То, что тесты не срабатывают — это нормально. Они для того и нужны, чтобы давать мне red bar, пока я не удовлетворю условиям теста. А сами условия написаны выше, и они, согласитесь, правильные?

«Ну и удар ниже пояса:»
Не прошел. Да, в данном случае нам это не удалось, но и не все же ситуации идеальны, правда?

Ну и обратите внимание, что, на самом деле, весь код, связанный с конкурентностью, сведен на примитив Monitor.
Что-то у нас разговор сваливается в непонятное русло («в интернете кто-то не прав» с обоих сторон). Предлагаю остаться при своих. Что бы быть честным выкладываю свой только что сделанный вариант решения, для первых трех тестов (и мой вариант этих тестов соответственно). Рабочий больше и сложнее. Поругаем его немного и разойдемся.

		[TestMethod]
		public void ExecTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				DateTime add = DateTime.Now;
				var execLog = new List<DateTime>();

				Action addTask = () =>
					disp.Add(TimeSpan.FromSeconds(1),
					() =>
					{
						execLog.Add(DateTime.Now);
						Breakpoint.Define("exec");
					});

				new ThreadTestManager(TimeSpan.FromSeconds(2), addTask).Run("exec");

				Assert.IsTrue(execLog.Single() - add > TimeSpan.FromSeconds(1));
			}
		}

		[TestMethod]
		public void ExecTwoTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				var log = new List<int>();

				Action act = () =>
					{
						disp.Add(TimeSpan.FromSeconds(1), () => { log.Add(0); Breakpoint.Define("exec0"); });
						Breakpoint.Define("pause");
						disp.Add(TimeSpan.FromSeconds(1), () => { log.Add(1); Breakpoint.Define("exec1"); });
					};

				new ThreadTestManager(TimeSpan.FromSeconds(3), act).Run("exec0", "pause", "exec1");

				Assert.AreEqual(2, log.Count);
				Assert.AreEqual(0, log[0]);
				Assert.AreEqual(1, log[1]);
			}
		}

		[TestMethod]
		public void ReplaceTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				var log = new List<int>();

				Action act = () =>
				{
					disp.Add(TimeSpan.FromSeconds(1), () => { log.Add(0); Breakpoint.Define("exec"); });
					disp.Add(TimeSpan.FromSeconds(1), () => { log.Add(1); Breakpoint.Define("exec"); });
				};

				new ThreadTestManager(TimeSpan.FromSeconds(3), act).Run(
					"exec", new BreakMark("exec") { Timeout = true });

				Assert.AreEqual(1, log.Count);
				Assert.AreEqual(1, log[0]);
			}
		}

	class TaskDispatcher : IDisposable
	{
		EventWaitHandle m_oldCancelEv;

		public TaskDispatcher()
		{
			m_oldCancelEv = new AutoResetEvent(false);
		}

		void Run(EventWaitHandle cancelEv, TimeSpan delay, Action act)
		{
			if (!cancelEv.WaitOne(delay))
				act();
		}

		public void Add(TimeSpan delay, Action act)
		{
			m_oldCancelEv.Set();

			var newCancelEv = new AutoResetEvent(false);

			var task = new Task(() => Run(newCancelEv, delay.Ticks > 0 ? delay : TimeSpan.Zero, act));
			task.Start();

			m_oldCancelEv = newCancelEv;
		}

		public void Dispose()
		{
			m_oldCancelEv.Set();
		}
	}
Кода (в тестах) больше, а читается он хуже. И еще и сильно завязан на текущее время (и не учитывает погрешности).
Грубо говоря, я никак не могу понять, что вы делаете в своем тесте. А это признак плохого теста.

А с другой стороны, мне кажется, что вы всю эту часть дискуссии никак не можете услышать мой тезис о том, что при обсуждении TDD важно то, как устроен тест, и что он тестирует, а не внутренности тестируемого класса.
Не буду спорить. Спасибо за критику и терпение
Кстати, а не покажете, как вы в вашем тесте тестируете коллизии?

(потому что, будем честными, больше в многопоточности тестировать нечего)
Изначально мы с вами выделили 5 тестов, а реализовывали только 3. Четвертый это: "«одновременно» добавить две задачи, удостовериться, что в очередь встала одна (любая, потому что поведение компонента не определено) из них". Самый простой способ сделать так:

		[TestMethod]
		public void SyncCancelPrevTaskTest()
		{
			using (var disp = new TaskDispatcher())
			{
				Action act0 = () => disp.Add(TimeSpan.Zero, () => { });
				Action act1 = () => disp.Add(TimeSpan.Zero, () => { });

				new ThreadTestManager(TimeSpan.FromSeconds(1), act0, act1).Run(
					new BreakMark(act0, "precancel prev task"),
					new BreakMark(act1, "precancel prev task") { Timeout = true },
					new BreakMark(act0, "finish add"));
			}
		}


Изменения в Add:

	class TaskDispatcher : IDisposable
	{
		object m_syncAdd = new object();
		...
		public void Add(TimeSpan delay, Action act)
		{
			lock (m_syncAdd)
			{
				Breakpoint.Define("precancel prev task");

				m_oldCancelEv.Set();

				var newCancelEv = new AutoResetEvent(false);

				var task = new Task(() => Run(newCancelEv, delay.Ticks > 0 ? delay : TimeSpan.Zero, act));
				task.Start();

				m_oldCancelEv = newCancelEv;

				Breakpoint.Define("finish add");
			}
		}
	}
Я не вижу в вашем тесте проверки, что выполнился действительно только один экземпляр. А так же, собственно, гарантии коллизии (то есть, того факта, что второй экземпляр был вызван до того, как мы вышли из первого).

Впрочем, существенно важнее то, что ради проверки вам пришлось модифицировать код class under test (причем ничем, кроме проверки, это не обосновано). А это, как неоднократно говорилось, очень плохо.
«того факта, что второй экземпляр был вызван до того, как мы вышли из первого»

	new BreakMark(act0, "precancel prev task"),
	new BreakMark(act1, "precancel prev task") { Timeout = true },
	new BreakMark(act0, "finish add")


Вторая строчка утверждает недостижимость «precancel prev task» для act1, третья — препятствует выходу из «finish add» (6, 7 свойство автоматических брейкпоинтов, конец статьи).

Большинство финишных Assert проверок из нашего разговора добавлены исключительно для вас. В боевых тестах они встречаются редко. В основном для проверки множественного входа. Обычно я удовлетворяюсь проходимостью сценария, заданного последовательностью брейкпоинтов.

По поводу «модифицировать код class under test» — мне известно. Обратите внимание первые три теста обошлись без этого. Именно для ослабления эффекта Breakpoint.Define помечен [Conditional(«DEBUG»)], а класс Breakpoint вынесен в отдельную сборку (приложенный Solution). По «модифицировать код class under test» можно обсудить. Вопрос тоже не такой простой, хотя что-то новое мы вряд ли скажем. Но вдруг…

DEBUG вы уже ругали. Дискуссия о нем заведет нас в такие дебри, что и трех веток будет мало. Если хотите, можем об этом поговорить отдельно (например в отдельном блоге — думаю количество участников и дельты рейтингов зашкалят).
«Вторая строчка утверждает недостижимость «precancel prev task» для act1»
Давайте попробуем прочитать ваши три строчки.

«Отпустить „precancel prev task“ в потоке 0»
«Упасть, если поток 1 достиг „precancel prev task“»

(очень, очень логичная запись в коде, правда? Вообще-то, как раз для «упасть, если» используются Assert)

А вот теперь вопрос: что мешает коду в потоке 0 успеть пройти всю линию выполнения до момента строчки 2?

«Большинство финишных Assert проверок из нашего разговора добавлены исключительно для вас. В боевых тестах они встречаются редко.»
А тут вы нарушаете AAA. Что, в общем-то, в TDD не так уж и хорошо.

«Обычно я удовлетворяюсь проходимостью сценария, заданного последовательностью брейкпоинтов.»
… а надо проверять сценарий работы. В частности, в вашем случае не надо проверять, что второй поток достиг брейкпойнта (это лишнее знание о внутренностях кода), надо проверять, что не было добавления в очередь.
1. Логичность названия:
У слова «timeout» — нет перевода «упасть». Первый аргумент в конструкторе называется timeout (и означает именно то как переводится), и он связязан с timeout-ом из BreakMark (на этом шаге мы свалимся по timeout-у). При выполнении сценария мы дожидаемся входа потока (конкретного или любого) в брейкпоинт и выпускаем его. Флаг BreakMark .Timeout, говорит что условий для отпускания не возникнет.

2. «что мешает коду в потоке 0 успеть пройти всю линию выполнения до момента строчки 2»
«третья — препятствует выходу из «finish add»». Пункт 7 свойств
Каждый следующий шаг сценария начинается, только после полного завершения предыдущего.

3. Одного шаблона AAA маловато, что бы начать заморачиваться Assert-ами. Шаблон по определению — это вариант, а не догма. Вот если бы мне вдруг понадобилось делать ветвления в сценарии (пока что это выглядит странным), то возможно я бы и занялся переделкой интерфейса.

4. «что не было добавления в очередь». В этом есть логика. Но это не отменит необходимости написать «finish add» (для блокировки выхода) и сделает необходимым добавление одного moc-описания (с обязательными интерфейсом и явной реализацией — породит две сущности, одна из которых будет располагаться в код under test).
«У слова «timeout» — нет перевода «упасть».»
Именно.

«Каждый следующий шаг сценария начинается, только после полного завершения предыдущего.»
Но при этом:
«При выполнении сценария мы дожидаемся входа потока (конкретного или любого) в брейкпоинт и выпускаем его.»
Но при этом брейкпойнт стоит внутри лока. Это значит, что (при корректном коде) вы просто никогда не попадете во второй шаг сценария.

Но хуже то, что это все из вашего теста не очевидно. А тест (особенно в TDD) должен быть не только проверкой чего-то, но еще и документацией поведения.

Непонятно ни что вы тестируете, ни как оно должно себя вести. А именно для этого создан шаблон AAA, который вы так легко отвергаете:

«Одного шаблона AAA маловато, что бы начать заморачиваться Assert-ами»
Теста без проверки не бывает. Иначе какой это тест?
«Это значит, что (при корректном коде) вы просто никогда не попадете во второй шаг сценария.»

Вы правы в этом и смысл проверки — коллизии нет, потому что в одно время этот код обрабатывает только один поток. Если же шаг с timeout-ум выполнится — будет исключение.
А исключение-то кто сгенерит?

Из _теста_ это не очевидно.

Повторю еще раз. То, что вы делаете — это не TDD. Вы написали для себя утилиту, которая что-то там симулирует в многопоточной системе. Ну хорошо. ваше право.

Но TDD — это описать поведение SUT самим тестом, а потом его реализовать. У вас же тест написан исходя из конкретных внутренностей SUT, и проверяет не поведение, а ваши допущения относительно этих самых внутренностей («если мы сюда не пришли, значит, поведение правильное»).
Читал в литературе. Общий принцип для тестирвания и отладки параллельно выполняющихся задач состоит в расстановке в проблемых местах метак выполняющие случайную задержку скажим 0...1000 ms.
И запускается программа, наблюдаем не появились ли в базе дубликаты, не пропущены ли какие-то элементы и другие косвенные признаки не правильной работы программы.
Если проблема проявилась нужно попробовать определить при каких задержках на каких метках она проявляется. Возможно понадобится нейкий менеджер отслеживающий задержки на метках и прочие хитрости.
Сам на практике пока такой приём не довелось применить, если кто пользовал раскажите, как оно в деле???

Чисто теоретически. Многократно запускаемый автоматический тест, который будет выполнять код с этими случайными проверками и контролировать корректность. Входные данные теста логируются (скажем, та же задержка) тем или иным способом, и если тест валится, то будет ясно, на чем конкретно. Ну и так далее…
Sign up to leave a comment.

Articles

Change theme settings