From c2df1e8d684900e3f262fd089d137c1eb0d7da83 Mon Sep 17 00:00:00 2001
From: Mark Harrah <dmharrah@gmail.com>
Date: Tue, 16 Jul 2013 13:55:10 -0400
Subject: [PATCH] Deduplicate options processing for forked/non-forked tests.

This in particular fixes testOnly/testQuick when forking.
---
 .../src/main/scala/sbt/ForkTests.scala        | 16 +++---------
 main/actions/src/main/scala/sbt/Tests.scala   | 25 +++++++++++--------
 sbt/src/sbt-test/tests/junit/test             |  3 +++
 3 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/main/actions/src/main/scala/sbt/ForkTests.scala b/main/actions/src/main/scala/sbt/ForkTests.scala
index ee6ed1913..4bbe95c5a 100755
--- a/main/actions/src/main/scala/sbt/ForkTests.scala
+++ b/main/actions/src/main/scala/sbt/ForkTests.scala
@@ -12,19 +12,11 @@ import ForkMain._
 
 private[sbt] object ForkTests {
 	def apply(runners: Map[TestFramework, Runner],  tests: List[TestDefinition], config: Execution, classpath: Seq[File], fork: ForkOptions, log: Logger): Task[TestOutput]  = {
-		val opts = config.options.toList
-		val listeners = opts flatMap {
-			case Listeners(ls) => ls
-			case _ => Nil
-		}
-		val testListeners = listeners flatMap {
+		val opts = processOptions(config, tests, log)
+		val testListeners = opts.testListeners flatMap {
 			case tl: TestsListener => Some(tl)
 			case _ => None
 		}
-		val filters = opts flatMap {
-			case Filter(f) => Some(f)
-			case _ => None
-		}
 
 		std.TaskExtra.task {
 			if (!tests.isEmpty) {
@@ -45,7 +37,7 @@ private[sbt] object ForkTests {
 						try {
 							os.writeBoolean(log.ansiCodesSupported)
 							
-							val testsFiltered = tests.filter(test => filters.forall(_(test.name))).map{
+							val testsFiltered = opts.tests.map{
 								t => new TaskDef(t.name, forkFingerprint(t.fingerprint), t.explicitlySpecified, t.selectors)
 							}.toArray
 							os.writeObject(testsFiltered)
@@ -59,7 +51,7 @@ private[sbt] object ForkTests {
 							}
 							os.flush()
 
-							(new React(is, os, log, listeners, resultsAcc)).react()
+							(new React(is, os, log, opts.testListeners, resultsAcc)).react()
 						} finally {
 							is.close();	os.close(); socket.close()
 						}
diff --git a/main/actions/src/main/scala/sbt/Tests.scala b/main/actions/src/main/scala/sbt/Tests.scala
index d6eb4118a..d2276cf24 100644
--- a/main/actions/src/main/scala/sbt/Tests.scala
+++ b/main/actions/src/main/scala/sbt/Tests.scala
@@ -105,7 +105,13 @@ object Tests
 	/** A named group of tests configured to run in the same JVM or be forked. */
 	final case class Group(name: String, tests: Seq[TestDefinition], runPolicy: TestRunPolicy)
 
-	def apply(frameworks: Map[TestFramework, Framework], testLoader: ClassLoader, runners: Map[TestFramework, Runner], discovered: Seq[TestDefinition], config: Execution, log: Logger): Task[Output] =
+	private[sbt] final class ProcessedOptions(
+		val tests: Seq[TestDefinition], 
+		val setup: Seq[ClassLoader => Unit],
+		val cleanup: Seq[ClassLoader => Unit],
+		val testListeners: Seq[TestReportListener]
+	)
+	private[sbt] def processOptions(config: Execution, discovered: Seq[TestDefinition], log: Logger): ProcessedOptions =
 	{
 			import collection.mutable.{HashSet, ListBuffer, Map, Set}
 		val testFilters = new ListBuffer[String => Boolean]
@@ -125,15 +131,6 @@ object Tests
 				case Listeners(listeners) => testListeners ++= listeners
 				case Setup(setupFunction) => setup += setupFunction
 				case Cleanup(cleanupFunction) => cleanup += cleanupFunction
-				/**
-					* There are two cases here.
-					* The first handles TestArguments in the project file, which
-					* might have a TestFramework specified.
-					* The second handles arguments to be applied to all test frameworks.
-					*   -- arguments from the project file that didnt have a framework specified
-					*   -- command line arguments (ex: test-only someClass -- someArg)
-					*      (currently, command line args must be passed to all frameworks)
-					*/
 				case a: Argument => // now handled by whatever constructs `runners`
 			}
 		}
@@ -146,7 +143,13 @@ object Tests
 		def includeTest(test: TestDefinition) = !excludeTestsSet.contains(test.name) && testFilters.forall(filter => filter(test.name))
 		val filtered0 = discovered.filter(includeTest).toList.distinct
 		val tests = if(orderedFilters.isEmpty) filtered0 else orderedFilters.flatMap(f => filtered0.filter(d => f(d.name))).toList.distinct
-		testTask(testLoader, frameworks, runners, tests, setup.readOnly, cleanup.readOnly, log, testListeners.readOnly, config)
+		new ProcessedOptions(tests, setup.toList, cleanup.toList, testListeners.toList)
+	}
+
+	def apply(frameworks: Map[TestFramework, Framework], testLoader: ClassLoader, runners: Map[TestFramework, Runner], discovered: Seq[TestDefinition], config: Execution, log: Logger): Task[Output] =
+	{
+		val o = processOptions(config, discovered, log)
+		testTask(testLoader, frameworks, runners, o.tests, o.setup, o.cleanup, log, o.testListeners, config)
 	}
 
 	def testTask(loader: ClassLoader, frameworks: Map[TestFramework, Framework], runners: Map[TestFramework, Runner], tests: Seq[TestDefinition],
diff --git a/sbt/src/sbt-test/tests/junit/test b/sbt/src/sbt-test/tests/junit/test
index ef48c9d40..b1cd8cbc4 100644
--- a/sbt/src/sbt-test/tests/junit/test
+++ b/sbt/src/sbt-test/tests/junit/test
@@ -9,3 +9,6 @@ $ copy-file changes/Failure.scala src/test/scala/Failure.scala
 -> test
 -> test-only com.foo.junit.test.blah.Failure
 > test-only com.foo.junit.test.blah.Success
+
+> set fork in Test := true
+> test-only com.foo.junit.test.blah.Success