Skip to content

Commit bf6ce11

Browse files
vsalvisvsalvis
committed
More robust partest/test switching for concurrent sbt instances
1 parent be2eaf0 commit bf6ce11

File tree

4 files changed

+56
-58
lines changed

4 files changed

+56
-58
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,5 @@ classes/
2828

2929
# Partest
3030
tests/partest-generated/
31+
tests/locks/
3132
/test-classes/

‎project/Build.scala‎

Lines changed: 38 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ object DottyBuild extends Build {
1717
// "-XX:+HeapDumpOnOutOfMemoryError", "-Xmx1g", "-Xss2m"
1818
)
1919

20-
var partestLock: FileLock = null
21-
2220
val defaults = Defaults.defaultSettings ++ Seq(
2321
scalaVersion in Global := "2.11.5",
2422
version in Global := "0.1-SNAPSHOT",
@@ -63,25 +61,18 @@ object DottyBuild extends Build {
6361

6462
// enable verbose exception messages for JUnit
6563
testOptions in Test += Tests.Argument(TestFrameworks.JUnit, "-a", "-v", "--run-listener=test.ContextEscapeDetector"),
66-
testOptions in Test += Tests.Cleanup({ () => if (partestLock != null) partestLock.release }),
67-
// When this file is locked, running test generates the files for partest.
68-
// Otherwise it just executes the tests directly. So running two separate
69-
// sbt instances might result in unexpected behavior if one does partest
70-
// and the other test, since the second still sees the locked file and thus
71-
// generates partest files instead of running JUnit tests, but doesn't
72-
// partest them.
64+
testOptions in Test += Tests.Cleanup({ () => partestLockFile.delete }),
65+
7366
lockPartestFile := {
74-
val partestLockFile = "." + File.separator + "tests" + File.separator + "partest.lock"
75-
try {
76-
partestLock = new RandomAccessFile(partestLockFile, "rw").getChannel.tryLock
77-
if (partestLock == null)
78-
throw new RuntimeException("ERROR: sbt partest: file is locked already. Bad things happen when trying to mix test/partest in two concurrent sbt instances.")
79-
} catch {
80-
case ex: java.nio.channels.OverlappingFileLockException => // locked already, Tests.Cleanup didn't run
81-
if (partestLock != null)
82-
partestLock.release
83-
throw new RuntimeException("ERROR: sbt partest: file was still locked, please try again or restart sbt.")
84-
}
67+
// When this file is present, running `test` generates the files for
68+
// partest. Otherwise it just executes the tests directly.
69+
val lockDir = partestLockFile.getParentFile
70+
lockDir.mkdirs
71+
// Cannot have concurrent partests as they write to the same directory.
72+
if (lockDir.list.size > 0)
73+
throw new RuntimeException("ERROR: sbt partest: another partest is already running, pid in lock file: " + lockDir.list.toList.mkString(" "))
74+
partestLockFile.createNewFile
75+
partestLockFile.deleteOnExit
8576
},
8677
runPartestRunner <<= Def.taskDyn {
8778
val jars = Seq((packageBin in Compile).value.getAbsolutePath) ++
@@ -99,29 +90,29 @@ object DottyBuild extends Build {
9990

10091
// http://grokbase.com/t/gg/simple-build-tool/135ke5y90p/sbt-setting-jvm-boot-paramaters-for-scala
10192
javaOptions <++= (managedClasspath in Runtime, packageBin in Compile) map { (attList, bin) =>
102-
// put the Scala {library, reflect} in the classpath
103-
val path = for {
104-
file <- attList.map(_.data)
105-
path = file.getAbsolutePath
106-
} yield "-Xbootclasspath/p:" + path
107-
// dotty itself needs to be in the bootclasspath
108-
val fullpath = ("-Xbootclasspath/a:" + bin) :: path.toList
109-
// System.err.println("BOOTPATH: " + fullpath)
110-
111-
val travis_build = // propagate if this is a travis build
112-
if (sys.props.isDefinedAt(TRAVIS_BUILD))
113-
List(s"-D$TRAVIS_BUILD=${sys.props(TRAVIS_BUILD)}") ::: travisMemLimit
114-
else
115-
List()
116-
117-
val tuning =
118-
if (sys.props.isDefinedAt("Oshort"))
119-
// Optimize for short-running applications, see https://github.com/lampepfl/dotty/issues/222
120-
List("-XX:+TieredCompilation", "-XX:TieredStopAtLevel=1")
93+
// put the Scala {library, reflect} in the classpath
94+
val path = for {
95+
file <- attList.map(_.data)
96+
path = file.getAbsolutePath
97+
} yield "-Xbootclasspath/p:" + path
98+
// dotty itself needs to be in the bootclasspath
99+
val fullpath = ("-Xbootclasspath/a:" + bin) :: path.toList
100+
// System.err.println("BOOTPATH: " + fullpath)
101+
102+
val travis_build = // propagate if this is a travis build
103+
if (sys.props.isDefinedAt(TRAVIS_BUILD))
104+
List(s"-D$TRAVIS_BUILD=${sys.props(TRAVIS_BUILD)}") ::: travisMemLimit
105+
else
106+
List()
107+
108+
val tuning =
109+
if (sys.props.isDefinedAt("Oshort"))
110+
// Optimize for short-running applications, see https://github.com/lampepfl/dotty/issues/222
111+
List("-XX:+TieredCompilation", "-XX:TieredStopAtLevel=1")
121112
else
122113
List()
123114

124-
tuning ::: agentOptions ::: travis_build ::: fullpath
115+
("-DpartestParentID=" + pid) :: tuning ::: agentOptions ::: travis_build ::: fullpath
125116
}
126117
) ++ addCommandAlias("partest", ";test:compile;lockPartestFile;test:test;runPartestRunner")
127118

@@ -174,10 +165,14 @@ object DottyBuild extends Build {
174165
lazy val benchmarks = Project(id = "dotty-bench", settings = benchmarkSettings,
175166
base = file("bench")) dependsOn(dotty % "compile->test")
176167

177-
lazy val lockPartestFile = TaskKey[Unit]("lockPartestFile", "Creates the file lock on ./tests/partest.lock")
178-
lazy val runPartestRunner = TaskKey[Unit]("runPartestRunner", "Runs partests")
179-
lazy val partestDeps = SettingKey[Seq[ModuleID]]("partestDeps", "Finds jars for partest dependencies")
168+
// Partest tasks
169+
lazy val lockPartestFile = TaskKey[Unit]("lockPartestFile", "Creates the lock file at ./tests/locks/partest-<pid>.lock")
170+
val partestLockFile = new File("." + File.separator + "tests" + File.separator + "locks" + File.separator + s"partest-$pid.lock")
171+
val pid = java.lang.Long.parseLong(java.lang.management.ManagementFactory.getRuntimeMXBean().getName().split("@")(0))
180172

173+
lazy val runPartestRunner = TaskKey[Unit]("runPartestRunner", "Runs partest")
174+
175+
lazy val partestDeps = SettingKey[Seq[ModuleID]]("partestDeps", "Finds jars for partest dependencies")
181176
def getJarPaths(modules: Seq[ModuleID], ivyHome: Option[File]): Seq[String] = ivyHome match {
182177
case Some(home) =>
183178
modules.map({ module =>

‎test/test/CompilerTest.scala‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import org.junit.Test
1414
/** This class has two modes: it can directly run compiler tests, or it can
1515
* generate the necessary file structure for partest in the directory
1616
* DPConfig.testRoot. Both modes are regular JUnit tests. Which mode is used
17-
* depends on the existence of the tests/runPartest.flag file which is created
18-
* by sbt to trigger partest generation. Sbt can then run partest on the
19-
* generated sources.
17+
* depends on the existence of the tests/locks/partest-ppid.lock file which is
18+
* created by sbt to trigger partest generation. Sbt will then run partest on
19+
* the generated sources.
2020
*
2121
* Through overriding the partestableXX methods, tests can always be run as
2222
* JUnit compiler tests. Run tests cannot be run by JUnit, only by partest.
@@ -45,18 +45,20 @@ abstract class CompilerTest extends DottyTest {
4545
def partestableList(testName: String, files: List[String], args: List[String], xerrors: Int) = true
4646

4747
val generatePartestFiles = {
48-
val partestLockFile = "." + JFile.separator + "tests" + JFile.separator + "partest.lock"
49-
try {
50-
val partestLock = new RandomAccessFile(partestLockFile, "rw").getChannel.tryLock
51-
if (partestLock != null) { // file not locked by sbt -> don't generate partest
52-
partestLock.release
53-
false
54-
} else true
55-
} catch {
56-
// if sbt doesn't fork in Test, the tryLock request will throw instead of
57-
// returning null, because locks are per JVM, not per thread
58-
case ex: java.nio.channels.OverlappingFileLockException => true
59-
}
48+
/* Because we fork in test, the JVM in which this JUnit test runs has a
49+
* different pid from the one that started the partest. But the forked VM
50+
* receives the pid of the parent as system property. If the lock file
51+
* exists, the parent is requesting partest generation. This mechanism
52+
* allows one sbt instance to run test (JUnit only) and another partest.
53+
* We cannot run two instances of partest at the same time, because they're
54+
* writing to the same directories. The sbt lock file generation prevents
55+
* this.
56+
*/
57+
val pid = System.getProperty("partestParentID")
58+
if (pid == null)
59+
false
60+
else
61+
new JFile("." + JFile.separator + "tests" + JFile.separator + "locks" + JFile.separator + s"partest-$pid.lock").exists
6062
}
6163

6264
// Delete generated files from previous run

‎tests/partest.lock‎

Whitespace-only changes.

0 commit comments

Comments
 (0)