Clarify ResetSystemProperties trait inheritance ordering.

Author: JoshRosen

core/src/test/scala/org/apache/spark/scheduler/SparkListenerSuite.scala

@@ -28,8 +28,8 @@ import org.apache.spark.{LocalSparkContext, SparkContext}
 import org.apache.spark.executor.TaskMetrics
 import org.apache.spark.util.ResetSystemProperties
 
-class SparkListenerSuite extends FunSuite with ResetSystemProperties with LocalSparkContext
-  with Matchers with BeforeAndAfter with BeforeAndAfterAll  {
+class SparkListenerSuite extends FunSuite  with LocalSparkContext with Matchers with BeforeAndAfter
+  with BeforeAndAfterAll with ResetSystemProperties {
 
   /** Length of time to wait while draining listener events. */
   val WAIT_TIMEOUT_MILLIS = 10000

core/src/test/scala/org/apache/spark/storage/BlockManagerSuite.scala

@@ -33,7 +33,7 @@ import akka.util.Timeout
 
 import org.mockito.Mockito.{mock, when}
 
-import org.scalatest.{BeforeAndAfter, FunSuite, Matchers, PrivateMethodTester}
+import org.scalatest._
 import org.scalatest.concurrent.Eventually._
 import org.scalatest.concurrent.Timeouts._
 
@@ -47,8 +47,8 @@ import org.apache.spark.storage.BlockManagerMessages.BlockManagerHeartbeat
 import org.apache.spark.util._
 
 
-class BlockManagerSuite extends FunSuite with ResetSystemProperties with Matchers
-  with BeforeAndAfter with PrivateMethodTester {
+class BlockManagerSuite extends FunSuite with Matchers with BeforeAndAfterEach
+  with PrivateMethodTester with ResetSystemProperties {
 
   private val conf = new SparkConf(false)
   var store: BlockManager = null
@@ -78,7 +78,7 @@ class BlockManagerSuite extends FunSuite with ResetSystemProperties with Matcher
     manager
   }
 
-  before {
+  override def beforeEach(): Unit = {
     val (actorSystem, boundPort) = AkkaUtils.createActorSystem(
       "test", "localhost", 0, conf = conf, securityManager = securityMgr)
     this.actorSystem = actorSystem
@@ -99,7 +99,7 @@ class BlockManagerSuite extends FunSuite with ResetSystemProperties with Matcher
     SizeEstimator invokePrivate initialize()
   }
 
-  after {
+  override def afterEach(): Unit = {
     if (store != null) {
       store.stop()
       store = null

core/src/test/scala/org/apache/spark/util/AkkaUtilsSuite.scala

@@ -31,7 +31,7 @@ import org.apache.spark.storage.BlockManagerId
 /**
   * Test the AkkaUtils with various security settings.
   */
-class AkkaUtilsSuite extends FunSuite with ResetSystemProperties with LocalSparkContext {
+class AkkaUtilsSuite extends FunSuite with LocalSparkContext with ResetSystemProperties {
 
   test("remote fetch security bad password") {
     val conf = new SparkConf

core/src/test/scala/org/apache/spark/util/ResetSystemProperties.scala

@@ -19,23 +19,39 @@ package org.apache.spark.util
 
 import java.util.Properties
 
-import org.scalatest.{Suite, SuiteMixin}
-
+import org.scalatest.{BeforeAndAfterEach, Suite}
 
 /**
  * Mixin for automatically resetting system properties that are modified in ScalaTest tests.
  * This resets the properties after each individual test.
  *
+ * The order in which fixtures are mixed in affects the order in which they are invoked by tests.
+ * If we have a suite `MySuite extends FunSuite with Foo with Bar`, then
+ * Bar's `super` is Foo, so Bar's beforeEach() will and afterEach() methods will be invoked first
+ * by the rest runner.
+ *
+ * This means that ResetSystemProperties should appear as the last trait in test suites that it's
+ * mixed into in order to ensure that the system properties snapshot occurs as early as possible.
+ * ResetSystemProperties calls super.afterEach() before performing its own cleanup, ensuring that
+ * the old properties are restored as late as possible.
+ *
  * See the "Composing fixtures by stacking traits" section at
  * http://www.scalatest.org/user_guide/sharing_fixtures for more details about this pattern.
  */
-private[spark] trait ResetSystemProperties extends SuiteMixin { this: Suite =>
-  abstract override def withFixture(test: NoArgTest) = {
-    val oldProperties = new Properties(System.getProperties)
+private[spark] trait ResetSystemProperties extends BeforeAndAfterEach { this: Suite =>
+  var oldProperties: Properties = null
+
+  override def beforeEach(): Unit = {
+    oldProperties = new Properties(System.getProperties)
+    super.beforeEach()
+  }
+
+  override def afterEach(): Unit = {
     try {
-      super.withFixture(test)
+      super.afterEach()
     } finally {
       System.setProperties(oldProperties)
+      oldProperties = null
     }
   }
 }

core/src/test/scala/org/apache/spark/util/SizeEstimatorSuite.scala

@@ -44,7 +44,7 @@ class DummyString(val arr: Array[Char]) {
 }
 
 class SizeEstimatorSuite
-  extends FunSuite with ResetSystemProperties with BeforeAndAfterEach with PrivateMethodTester {
+  extends FunSuite with BeforeAndAfterEach with PrivateMethodTester with ResetSystemProperties {
 
   override def beforeEach() {
     // Set the arch to 64-bit and compressedOops to true to get a deterministic test-case