Skip to content

Commit b29d802

Browse files
committed
Rewritten SuiteSortingReporter and TestSortingReporter to create timer instance only when required, avoiding creating the timer thread when it is not needed.
1 parent 9d8061c commit b29d802

File tree

2 files changed

+41
-21
lines changed

2 files changed

+41
-21
lines changed

scalatest/src/main/scala/org/scalatest/tools/SuiteSortingReporter.scala

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ private[scalatest] class SuiteSortingReporter(dispatch: Reporter, val testSortin
4545
}
4646
}
4747

48-
private val timer = new Timer
49-
private var timeoutTask: Option[TimeoutTask] = None
48+
private var timeoutTask: Option[(TimeoutTask, Timer)] = None
5049

5150
def doApply(event: Event): Unit = {
5251
synchronized {
@@ -177,6 +176,8 @@ private[scalatest] class SuiteSortingReporter(dispatch: Reporter, val testSortin
177176
slotListBuf = fireReadySuiteEvents(slotListBuf.tail)
178177
if (slotListBuf.size > 0)
179178
scheduleTimeoutTask()
179+
else
180+
cancelTimeoutTask()
180181
}
181182
}
182183
}
@@ -283,23 +284,38 @@ private[scalatest] class SuiteSortingReporter(dispatch: Reporter, val testSortin
283284
private def scheduleTimeoutTask(): Unit = {
284285
val head = slotListBuf.head // Assumes waitingBuffer is non-empty. Put a require there to make that obvious.
285286
timeoutTask match {
286-
case Some(task) =>
287-
if (head.suiteId != task.slot.suiteId) {
288-
task.cancel()
289-
timeoutTask = Some(new TimeoutTask(head)) // Replace the old with the new
290-
timer.schedule(timeoutTask.get, testSortingTimeout.millisPart)
287+
case Some((oldTask, oldTimer)) =>
288+
if (head.suiteId != oldTask.slot.suiteId) {
289+
oldTask.cancel()
290+
oldTimer.cancel()
291+
val (task, timer) = (new TimeoutTask(head), new Timer)
292+
timeoutTask = Some((task, timer)) // Replace the old with the new
293+
timer.schedule(task, testSortingTimeout.millisPart)
291294
}
292295
case None =>
293-
timeoutTask = Some(new TimeoutTask(head)) // Just create a new one
294-
timer.schedule(timeoutTask.get, testSortingTimeout.millisPart)
296+
val (task, timer) = (new TimeoutTask(head), new Timer)
297+
timeoutTask = Some((task, timer)) // Just create a new one
298+
timer.schedule(task, testSortingTimeout.millisPart)
295299
}
296300
}
301+
302+
// Also happening inside synchronized block
303+
private def cancelTimeoutTask(): Unit = {
304+
timeoutTask match { // Waiting buffer is zero, so no timeout needed
305+
case Some((task, timer)) =>
306+
task.cancel()
307+
timer.cancel()
308+
timeoutTask = None
309+
case None =>
310+
}
311+
}
297312

298313
private def timeout(): Unit = {
299314
synchronized {
300315
if (slotListBuf.size > 0) {
301316
val head = slotListBuf.head
302-
if (timeoutTask.get.slot.suiteId == head.suiteId) { // Probably a double check, or just in case there's race condition
317+
val (task, _) = timeoutTask.get
318+
if (task.slot.suiteId == head.suiteId) { // Probably a double check, or just in case there's race condition
303319
val newSlot = head.copy(ready = true) // Essentially, if time out, just say that one is ready. This test's events go out, and
304320
slotListBuf.update(0, newSlot)
305321
}

scalatest/src/main/scala/org/scalatest/tools/TestSortingReporter.scala

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ private[scalatest] class TestSortingReporter(suiteId: String, dispatch: Reporter
5050
}
5151
}
5252

53-
private val timer = new Timer
54-
private var timeoutTask: Option[TimeoutTask] = None
53+
private var timeoutTask: Option[(TimeoutTask, Timer)] = None
5554

5655
/**
5756
* Called to indicate a test is being distributed. The tests will be reported
@@ -259,23 +258,27 @@ private[scalatest] class TestSortingReporter(suiteId: String, dispatch: Reporter
259258
private def scheduleTimeoutTask(): Unit = {
260259
val head = waitingBuffer.head // Assumes waitingBuffer is non-empty. Put a require there to make that obvious.
261260
timeoutTask match {
262-
case Some(task) =>
263-
if (head.uuid != task.slot.uuid) {
264-
task.cancel()
265-
timeoutTask = Some(new TimeoutTask(head)) // Replace the old with the new
266-
timer.schedule(timeoutTask.get, sortingTimeout.millisPart)
261+
case Some((oldTask, oldTimer)) =>
262+
if (head.uuid != oldTask.slot.uuid) {
263+
oldTask.cancel()
264+
oldTimer.cancel()
265+
val (task, timer) = (new TimeoutTask(head), new Timer)
266+
timeoutTask = Some((task, timer)) // Replace the old with the new
267+
timer.schedule(task, sortingTimeout.millisPart)
267268
}
268269
case None =>
269-
timeoutTask = Some(new TimeoutTask(head)) // Just create a new one
270-
timer.schedule(timeoutTask.get, sortingTimeout.millisPart)
270+
val (task, timer) = (new TimeoutTask(head), new Timer)
271+
timeoutTask = Some((task, timer)) // Just create a new one
272+
timer.schedule(task, sortingTimeout.millisPart)
271273
}
272274
}
273275

274276
// Also happening inside synchronized block
275277
private def cancelTimeoutTask(): Unit = {
276278
timeoutTask match { // Waiting buffer is zero, so no timeout needed
277-
case Some(task) =>
279+
case Some((task, timer)) =>
278280
task.cancel()
281+
timer.cancel()
279282
timeoutTask = None
280283
case None =>
281284
}
@@ -285,7 +288,8 @@ private[scalatest] class TestSortingReporter(suiteId: String, dispatch: Reporter
285288
synchronized {
286289
if (waitingBuffer.size > 0) {
287290
val head = waitingBuffer.head
288-
if (timeoutTask.get.slot.uuid == head.uuid) { // Probably a double check, or just in case there's race condition
291+
val (task, _) = timeoutTask.get
292+
if (task.slot.uuid == head.uuid) { // Probably a double check, or just in case there's race condition
289293
val newSlot = head.copy(ready = true) // Essentially, if time out, just say that one is ready. This test's events go out, and
290294
waitingBuffer.update(0, newSlot)
291295
}

0 commit comments

Comments
 (0)