Merge pull request #10690 from som-snytt/audit/isWin

Memoize osName properties and use assumeFalse, noWin

src/compiler/scala/tools/nsc/backend/jvm/ClassfileWriters.scala

@@ -235,9 +235,9 @@ abstract class ClassfileWriters {
   }
 
   private final class DirEntryWriter(base: Path) extends FileWriter {
+    import scala.util.Properties.{isWin => isWindows}
     val builtPaths = new ConcurrentHashMap[Path, java.lang.Boolean]()
     val noAttributes = Array.empty[FileAttribute[_]]
-    private val isWindows = scala.util.Properties.isWin
 
     private def checkName(component: Path): Unit = if (isWindows) {
       val specials = raw"(?i)CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9]".r

src/library/scala/util/Properties.scala

@@ -130,16 +130,16 @@ private[scala] trait PropertiesTrait {
 
   /* Some derived values. */
   /** Returns `true` iff the underlying operating system is a version of Microsoft Windows. */
-  def isWin                 = osName startsWith "Windows"
+  lazy val isWin            = osName.startsWith("Windows")
   // See https://mail.openjdk.java.net/pipermail/macosx-port-dev/2012-November/005148.html for
   // the reason why we don't follow developer.apple.com/library/mac/#technotes/tn2002/tn2110.
   /** Returns `true` iff the underlying operating system is a version of Apple Mac OSX.  */
-  def isMac                 = osName startsWith "Mac OS X"
+  lazy val isMac            = osName.startsWith("Mac OS X")
   /** Returns `true` iff the underlying operating system is a Linux distribution. */
-  def isLinux               = osName startsWith "Linux"
+  lazy val isLinux          = osName.startsWith("Linux")
 
   /* Some runtime values. */
-  private[scala] def isAvian = javaVmName contains "Avian"
+  private[scala] lazy val isAvian = javaVmName.contains("Avian")
 
   private[scala] def coloredOutputEnabled: Boolean = propOrElse("scala.color", "auto") match {
     case "auto" => System.console() != null && !isWin

src/partest/scala/tools/partest/nest/Runner.scala

@@ -24,9 +24,10 @@ import scala.concurrent.duration.Duration
 import scala.reflect.internal.FatalError
 import scala.reflect.internal.util.ScalaClassLoader, ScalaClassLoader.URLClassLoader
 import scala.sys.process.{Process, ProcessLogger}
-import scala.tools.nsc.Properties.{isWin, propOrEmpty}
+import scala.tools.nsc.Properties.{isAvian, isWin, javaSpecVersion, propOrEmpty}
 import scala.tools.nsc.{CompilerCommand, Global, Settings}
 import scala.tools.nsc.reporters.ConsoleReporter
+import scala.tools.nsc.settings.ScalaVersion
 import scala.tools.nsc.util.stackTraceString
 import scala.util.{Failure, Success, Try, Using}
 import scala.util.Properties.isJavaAtLeast
@@ -286,8 +287,6 @@ class Runner(val testInfo: TestInfo, val suiteRunner: AbstractRunner) {
    *  A missing flag evaluates the same as true.
    */
   def filteredCheck: Seq[String] = {
-    import scala.util.Properties.{javaSpecVersion, isAvian, isWin}
-    import scala.tools.nsc.settings.ScalaVersion
     // use lines in block with this label?
     def retainOn(expr0: String) = {
       val expr = expr0.trim

src/partest/scala/tools/partest/utils/Properties.scala

@@ -17,5 +17,5 @@ package utils
 object Properties extends scala.util.PropertiesTrait {
   protected def propCategory    = "scala-partest"
   protected def pickJarBasedOn  = classOf[nest.Runner]
-  override def isAvian = super.isAvian
+  override lazy val isAvian = javaVmName.contains("Avian")
 }

src/testkit/scala/tools/testkit/AssertUtil.scala

@@ -14,6 +14,7 @@ package scala.tools.testkit
 
 import org.junit.Assert.{assertEquals, assertNotEquals}
 import org.junit.Assert.{assertFalse, assertTrue}
+import org.junit.Assume.{assumeFalse, assumeTrue}
 
 import scala.annotation.nowarn
 import scala.collection.mutable
@@ -22,6 +23,7 @@ import scala.reflect.ClassTag
 import scala.runtime.BoxesRunTime
 import scala.runtime.ScalaRunTime.stringOf
 import scala.util.{Failure, Success, Try}
+import scala.util.Properties.isWin
 import scala.util.chaining._
 import scala.util.control.{ControlThrowable, NonFatal}
 import java.lang.ref.{Reference, ReferenceQueue, SoftReference}
@@ -45,14 +47,19 @@ object AssertUtil {
   // junit fail is Unit
   def fail(message: String): Nothing = throw new AssertionError(message)
 
+  def noWin(message: String = "skipping test on Windows")(body: => Unit) = {
+    assumeFalse(message, isWin)
+    body
+  }
+
   private val Bail = new ControlThrowable {}
 
   def bail(): Nothing = throw Bail
 
-  // maybe there is a way to communicate that the test was skipped
+  // if the test bails out, communicate a violated assumption
   def bailable(name: String)(test: => Unit): Unit =
     try test
-    catch { case _: Bail.type => println(s"$name skipped bail!") }
+    catch { case _: Bail.type => assumeTrue(s"$name skipped bail!", false) }
 
   private val printable = raw"\p{Print}".r
 

test/files/run/macro-openmacros/Impls_Macros_1.scala

@@ -1,6 +1,7 @@
 // scalac: -Yrangepos:false
 import scala.language.experimental.macros
 import scala.reflect.macros.blackbox.Context
+import scala.util.Properties.isWin
 
 object Macros {
   def impl(c: Context): c.Expr[Unit] = {
@@ -10,7 +11,6 @@ object Macros {
     def normalizePaths(s: String) = {
       val base = (dir.getCanonicalPath + java.io.File.separator).replace('\\', '/')
       var regex = """\Q%s\E""" format base
-      val isWin = System.getProperty("os.name", "") startsWith "Windows"
       if (isWin) regex = "(?i)" + regex
       s.replace('\\', '/').replaceAll(regex, "")
     }

test/junit/scala/sys/process/ProcessTest.scala

@@ -13,15 +13,13 @@ import scala.sys.process._ // test from outside the package to ensure implicits
 import scala.tools.testkit.AssertUtil._
 import scala.tools.testkit.ReleasablePath._
 import scala.util.Try
-import scala.util.Properties._
 import scala.util.Using
 import scala.util.chaining._
 
 import org.junit.Test
 import org.junit.Assert._
 
 class ProcessTest {
-  private def testily(body: => Unit) = if (!isWin) body
 
   private def withIn[A](in: InputStream)(body: => A): A = {
     val saved = System.in
@@ -34,7 +32,7 @@ class ProcessTest {
   // until after the latch, which is after the innocuous process exited,
   // and then attempt to close the process.getOutputStream, which is known
   // to be closed already (because that is how process exit was detected).
-  @Test def t7963(): Unit = testily {
+  @Test def t7963(): Unit = noWin() {
     var exception: Exception = null
     val latch: CountDownLatch = new CountDownLatch(1)
     val inputStream = new ByteArrayInputStream("a".getBytes) {
@@ -54,12 +52,12 @@ class ProcessTest {
     assertNull(exception)
   }
 
-  @Test def t10007(): Unit = testily {
+  @Test def t10007(): Unit = noWin() {
     val res = ("cat" #< new ByteArrayInputStream("lol".getBytes)).!!
     assertEquals("lol\n", res)
   }
   // test non-hanging
-  @Test def t10055(): Unit = testily {
+  @Test def t10055(): Unit = noWin() {
     val res = ("cat" #< ( () => -1 ) ).!
     assertEquals(0, res)
   }
@@ -69,13 +67,13 @@ class ProcessTest {
       (foo, bar) => assertEquals(0, Process.cat(Seq(foo, bar)).!)
     }
 
-  @Test def processApply(): Unit = testily {
+  @Test def processApply(): Unit = noWin() {
     Using.resources(File.createTempFile("foo", "tmp"), File.createTempFile("bar", "tmp")) {
       (foo, bar) => assertEquals(0, Process("cat", Seq(foo, bar).map(_.getAbsolutePath)).!)
     }
   }
 
-  @Test def t10696(): Unit = testily {
+  @Test def t10696(): Unit = noWin() {
     val res1 = Process("false").lazyLines
     assertEquals("LazyList(<not computed>)", res1.toString())
     val ex = Try(res1.head).failed.get
@@ -88,7 +86,7 @@ class ProcessTest {
 
   private def createFile(prefix: String) = createTempFile(prefix, "tmp").tap(f => Files.write(f, List(prefix).asJava, UTF_8))
 
-  @Test def t10823(): Unit = testily {
+  @Test def t10823(): Unit = noWin() {
     Using.resources(createFile("hello"), createFile("world"), createTempFile("out", "tmp")) { (file1, file2, out) =>
       val cat = Process.cat(List(file1, file2).map(_.toFile))
       val p = cat #> out.toFile
@@ -99,7 +97,7 @@ class ProcessTest {
   }
 
   // a test for A && B where A fails and B is not started
-  @Test def t10823_short_circuit(): Unit = testily {
+  @Test def t10823_short_circuit(): Unit = noWin() {
 
     val noFile = Paths.get("total", "junk")
     val p2     = new ProcessMock(error = false)

test/junit/scala/tools/nsc/QuickfixTest.scala

@@ -7,14 +7,15 @@ import java.nio.file.Files
 import scala.reflect.internal.util.BatchSourceFile
 import scala.reflect.io.AbstractFile
 import scala.tools.nsc.reporters.StoreReporter
+import scala.tools.testkit.AssertUtil._
 import scala.tools.testkit.BytecodeTesting
 import scala.tools.testkit.ReleasablePath._
 import scala.util.Using
 
 class QuickfixTest extends BytecodeTesting {
 
   def testQuickfixs(as: List[String], b: String, args: String, checkInfo: StoreReporter.Info => Boolean = _ => true): Unit =
-    if (!scala.util.Properties.isWin) {
+    noWin() {
       Using.resource(Files.createTempDirectory("quickfixTest")) { tmpDir =>
         val srcs = as.indices.map(i => tmpDir.resolve(s"unitSource$i.scala")).toList
         as.lazyZip(srcs).foreach { case (a, src) => Files.write(src, a.getBytes) }

test/junit/scala/tools/nsc/classpath/ZipAndJarFileLookupFactoryTest.scala

@@ -7,14 +7,14 @@ import java.net.{URI, URL}
 import java.nio.file._
 import java.nio.file.attribute.FileTime
 import scala.reflect.io.AbstractFile
+import scala.tools.testkit.AssertUtil._
 import scala.tools.testkit.ForDeletion
 import scala.tools.testkit.Releasables._
 import scala.util.chaining._
 import scala.util.Using
 
 class ZipAndJarFileLookupFactoryTest {
-  @Test def cacheInvalidation(): Unit = {
-    if (scala.util.Properties.isWin) return // can't overwrite an open file on windows.
+  @Test def cacheInvalidation(): Unit = noWin() { // can't overwrite an open file on windows.
 
     val f = Files.createTempFile("test-", ".jar")
     Files.delete(f)

test/junit/scala/tools/testkit/AssertUtilTest.scala

@@ -21,6 +21,7 @@ import AssertUtil._
 import java.lang.ref._
 
 import scala.annotation.unused
+import scala.util.Properties.isWin
 
 @RunWith(classOf[JUnit4])
 class AssertUtilTest {
@@ -143,4 +144,7 @@ class AssertUtilTest {
       assertSameElements(Array(17), Array(42))
     }
   }
+  @Test def `noWin skips test`: Unit = noWin() {
+    assertEquals(false, isWin)
+  }
 }