Remove BlockchainBuilder, speed up ChainHandler test that processes m… (#723)

* Remove BlockchainBuilder, speed up ChainHandler test that processes mainnet headers, add test for ChainAppConfig

* Add test for BlockHeaderDAO.getAncestorAtHeight()

* Consolidate ChainAppConfigTest, add ScalaTestUtil to help with working with scalatest, speed up ChainHandlerTest a bit more

bitcoind-rpc-test/src/test/scala/org/bitcoins/rpc/TestRpcUtilTest.scala

@@ -8,7 +8,7 @@ import org.bitcoins.rpc.client.common.RpcOpts.AddNodeArgument
 import org.bitcoins.rpc.util.AsyncUtil.RpcRetryException
 import org.bitcoins.rpc.util.{AsyncUtil, RpcUtil}
 import org.bitcoins.testkit.rpc.BitcoindRpcTestUtil
-import org.bitcoins.testkit.util.BitcoindRpcTest
+import org.bitcoins.testkit.util.{BitcoindRpcTest, FileUtil}
 
 import scala.concurrent.Future
 import scala.concurrent.duration.DurationInt
@@ -114,7 +114,7 @@ class TestRpcUtilTest extends BitcoindRpcTest {
     assert(dir.getPath().startsWith(scala.util.Properties.tmpDir))
     assert(
       dir.listFiles.contains(new File(dir.getAbsolutePath + "/bitcoin.conf")))
-    BitcoindRpcTestUtil.deleteTmpDir(dir)
+    FileUtil.deleteTmpDir(dir)
     assert(!dir.exists)
   }
 

chain-test/src/test/scala/org/bitcoins/chain/blockchain/ChainHandlerTest.scala

@@ -9,7 +9,7 @@ import org.bitcoins.chain.models.{
   BlockHeaderDbHelper
 }
 import org.bitcoins.core.protocol.blockchain.BlockHeader
-import org.bitcoins.testkit.util.FileUtil
+import org.bitcoins.testkit.util.{FileUtil, ScalaTestUtil}
 import org.bitcoins.testkit.chain.fixture.ChainFixtureTag
 import org.bitcoins.testkit.chain.{
   BlockHeaderHelper,
@@ -124,10 +124,9 @@ class ChainHandlerTest extends ChainUnitTest {
         val blockHeadersToTest = blockHeaders.tail
           .take(
             (2 * chainHandler.chainConfig.chain.difficultyChangeInterval + 1).toInt)
-          .toList
 
         processHeaders(processorF = processorF,
-                       remainingHeaders = blockHeadersToTest,
+                       headers = blockHeadersToTest,
                        height = ChainUnitTest.FIRST_POW_CHANGE + 1)
       }
   }
@@ -258,20 +257,33 @@ class ChainHandlerTest extends ChainUnitTest {
 
   final def processHeaders(
       processorF: Future[ChainApi],
-      remainingHeaders: List[BlockHeader],
+      headers: Vector[BlockHeader],
       height: Int): Future[Assertion] = {
-    remainingHeaders match {
-      case header :: headersTail =>
-        val newProcessorF = processorF.flatMap(_.processHeader(header))
-        val getHeaderF = newProcessorF.flatMap(_.getHeader(header.hashBE))
-        val expectedBlockHeaderDb =
-          BlockHeaderDbHelper.fromBlockHeader(height, header)
-        val assertionF =
-          getHeaderF.map(tips => assert(tips.contains(expectedBlockHeaderDb)))
-        assertionF.flatMap(_ =>
-          processHeaders(newProcessorF, headersTail, height = height + 1))
-      case Nil => succeed
+    val processedHeadersF = processorF.flatMap(_.processHeaders(headers))
+
+    def loop(
+        remainingHeaders: Vector[BlockHeader],
+        height: Int,
+        accum: Vector[Future[Assertion]]): Vector[Future[Assertion]] = {
+      remainingHeaders match {
+        case header +: headersTail =>
+          val getHeaderF = processedHeadersF.flatMap(_.getHeader(header.hashBE))
+          val expectedBlockHeaderDb =
+            BlockHeaderDbHelper.fromBlockHeader(height, header)
+          val assertionF =
+            getHeaderF.map(headerOpt =>
+              assert(headerOpt.contains(expectedBlockHeaderDb)))
+          val newAccum = accum.:+(assertionF)
+          loop(headersTail, height + 1, newAccum)
+        case Vector() =>
+          accum
+      }
     }
+
+    val vecFutAssert: Vector[Future[Assertion]] =
+      loop(headers, height, Vector.empty)
+
+    ScalaTestUtil.toAssertF(vecFutAssert)
   }
 
   /** Builds two competing headers that are built from the same parent */

chain-test/src/test/scala/org/bitcoins/chain/config/ChainAppConfigTest.scala

@@ -1,20 +1,39 @@
-package org.bitcoins.chain
+package org.bitcoins.chain.config
 
-import org.bitcoins.testkit.util.BitcoinSUnitTest
-import org.bitcoins.core.config.TestNet3
-import com.typesafe.config.Config
-import com.typesafe.config.ConfigFactory
-import org.bitcoins.core.config.RegTest
-import org.bitcoins.core.config.MainNet
-import org.bitcoins.chain.config.ChainAppConfig
 import java.nio.file.Files
-import ch.qos.logback.classic.Level
 
-class ChainAppConfigTest extends BitcoinSUnitTest {
+import akka.actor.ActorSystem
+import ch.qos.logback.classic.Level
+import com.typesafe.config.ConfigFactory
+import org.bitcoins.core.config.{MainNet, RegTest, TestNet3}
+import org.bitcoins.testkit.chain.ChainUnitTest
+import org.bitcoins.testkit.util.FileUtil
+import org.scalatest.FutureOutcome
+
+class ChainAppConfigTest extends ChainUnitTest {
   val tempDir = Files.createTempDirectory("bitcoin-s")
   val config = ChainAppConfig(directory = tempDir)
+  val chainAppConfig = appConfig
 
-  it must "be overridable" in {
+  implicit override val system = ActorSystem("ChainAppConfigTest")
+
+  behavior of "ChainAppConfig"
+
+  override def withFixture(test: OneArgAsyncTest): FutureOutcome =
+    withChainFixture(test)
+
+  it must "initialize our chain project" in { _ =>
+    val isInitF = chainAppConfig.isInitialized()
+
+    for {
+      isInit <- isInitF
+      _ = assert(!isInit)
+      _ <- chainAppConfig.initialize()
+      isInitAgain <- chainAppConfig.isInitialized()
+    } yield assert(isInitAgain)
+  }
+
+  it must "be overridable" in { _ =>
     assert(config.network == RegTest)
 
     val otherConf = ConfigFactory.parseString("bitcoin-s.network = testnet3")
@@ -26,7 +45,7 @@ class ChainAppConfigTest extends BitcoinSUnitTest {
     assert(mainnet.network == MainNet)
   }
 
-  it must "be overridable with multiple levels" in {
+  it must "be overridable with multiple levels" in { _ =>
     val testnet = ConfigFactory.parseString("bitcoin-s.network = testnet3")
     val mainnet = ConfigFactory.parseString("bitcoin-s.network = mainnet")
     val overriden: ChainAppConfig = config.withOverrides(testnet, mainnet)
@@ -34,20 +53,19 @@ class ChainAppConfigTest extends BitcoinSUnitTest {
 
   }
 
-  it must "have user data directory configuration take precedence" in {
-
+  it must "have user data directory configuration take precedence" in { _ =>
     val tempDir = Files.createTempDirectory("bitcoin-s")
     val tempFile = Files.createFile(tempDir.resolve("bitcoin-s.conf"))
     val confStr = """
-    | bitcoin-s {
-    |   network = testnet3
-    |   
-    |   logging {
-    |     level = off
-    |
-    |     p2p = warn
-    |   }
-    | }
+                    | bitcoin-s {
+                    |   network = testnet3
+                    |
+                    |   logging {
+                    |     level = off
+                    |
+                    |     p2p = warn
+                    |   }
+                    | }
     """.stripMargin
     val _ = Files.write(tempFile, confStr.getBytes())
 
@@ -58,4 +76,9 @@ class ChainAppConfigTest extends BitcoinSUnitTest {
     assert(appConfig.logLevel == Level.OFF)
     assert(appConfig.p2pLogLevel == Level.WARN)
   }
+
+  override def afterAll: Unit = {
+
+    FileUtil.deleteTmpDir(chainAppConfig.baseDatadir)
+  }
 }

chain-test/src/test/scala/org/bitcoins/chain/models/BlockHeaderDAOTest.scala

@@ -188,4 +188,17 @@ class BlockHeaderDAOTest extends ChainUnitTest {
       found <- foundF
     } yield assert(found.get == created)
   }
+
+  it must "get an ancestor at a specified height" in {
+    blockHeaderDAO: BlockHeaderDAO =>
+      val blockHeader = BlockHeaderHelper.buildNextHeader(genesisHeaderDb)
+      val createdF = blockHeaderDAO.create(blockHeader)
+
+      val genesisF = createdF.flatMap(created =>
+        blockHeaderDAO.getAncestorAtHeight(created, 0))
+
+      genesisF.map { genesisOpt =>
+        assert(genesisOpt.contains(genesisHeaderDb))
+      }
+  }
 }

chain/src/main/scala-2.11/org/bitcoins/chain/blockchain/BlockchainBuilder.scala

@@ -1,21 +0,0 @@
-package org.bitcoins.chain.blockchain
-
-import org.bitcoins.chain.models.{BlockHeaderDAO, BlockHeaderDb}
-
-import scala.collection.mutable
-
-case class BlockchainBuilder(blockHeaderDAO: BlockHeaderDAO)
-    extends mutable.Builder[BlockHeaderDb, Blockchain] {
-  private val internal = Vector.newBuilder[BlockHeaderDb]
-
-  override def result(): Blockchain = {
-    Blockchain.fromHeaders(internal.result().reverse)
-  }
-
-  override def +=(blockHeaderDb: BlockHeaderDb): this.type = {
-    internal.+=(blockHeaderDb)
-    this
-  }
-
-  override def clear(): Unit = internal.clear()
-}

chain/src/main/scala-2.12/org/bitcoins/chain/blockchain/BlockchainBuilder.scala

@@ -1,21 +0,0 @@
-package org.bitcoins.chain.blockchain
-
-import org.bitcoins.chain.models.{BlockHeaderDAO, BlockHeaderDb}
-
-import scala.collection.mutable
-
-case class BlockchainBuilder(blockHeaderDAO: BlockHeaderDAO)
-    extends mutable.Builder[BlockHeaderDb, Blockchain] {
-  private val internal = Vector.newBuilder[BlockHeaderDb]
-
-  override def result(): Blockchain = {
-    Blockchain.fromHeaders(internal.result().reverse)
-  }
-
-  override def +=(blockHeaderDb: BlockHeaderDb): this.type = {
-    internal.+=(blockHeaderDb)
-    this
-  }
-
-  override def clear(): Unit = internal.clear()
-}

testkit/src/main/scala/org/bitcoins/testkit/BitcoinSTestAppConfig.scala

@@ -4,6 +4,8 @@ import org.bitcoins.server.BitcoinSAppConfig
 import com.typesafe.config._
 import java.nio.file._
 
+import org.bitcoins.testkit.util.FileUtil
+
 object BitcoinSTestAppConfig {
 
   /**
@@ -64,4 +66,10 @@ object BitcoinSTestAppConfig {
     |""".stripMargin
     ConfigFactory.parseString(nestedConfStr)
   }
+
+  def deleteAppConfig(app: BitcoinSAppConfig): Boolean = {
+    FileUtil.deleteTmpDir(app.walletConf.baseDatadir) &&
+    FileUtil.deleteTmpDir(app.chainConf.baseDatadir) &&
+    FileUtil.deleteTmpDir(app.nodeConf.baseDatadir)
+  }
 }

testkit/src/main/scala/org/bitcoins/testkit/rpc/BitcoindRpcTestUtil.scala

@@ -5,7 +5,6 @@ import java.nio.file.Paths
 
 import akka.actor.ActorSystem
 import akka.stream.ActorMaterializer
-
 import org.bitcoins.core.config.RegTest
 import org.bitcoins.core.crypto.{
   DoubleSha256Digest,
@@ -52,6 +51,7 @@ import scala.concurrent.duration.{DurationInt, FiniteDuration}
 import scala.util._
 import org.bitcoins.rpc.config.BitcoindConfig
 import java.io.File
+
 import com.typesafe.config.Config
 import com.typesafe.config.ConfigFactory
 import java.nio.file.Path
@@ -60,6 +60,8 @@ import org.bitcoins.rpc.client.common.BitcoindVersion.V16
 import org.bitcoins.rpc.client.common.BitcoindVersion.V17
 import java.nio.file.Files
 
+import org.bitcoins.testkit.util.FileUtil
+
 //noinspection AccessorLikeMethodIsEmptyParen
 trait BitcoindRpcTestUtil extends BitcoinSLogger {
   import BitcoindRpcTestUtil.DEFAULT_LONG_DURATION
@@ -271,7 +273,7 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
 
     val serverStops = servers.map { s =>
       val stopF = s.stop()
-      deleteTmpDir(s.getDaemon.datadir)
+      FileUtil.deleteTmpDir(s.getDaemon.datadir)
       stopF.onComplete {
         case Failure(exception) =>
           logger.error(s"Could not shut down sever: $exception")
@@ -293,27 +295,6 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
     stopServers(Vector(server))
   }
 
-  /**
-    * Deletes the given temporary directory
-    *
-    * @throws IllegalArgumentException if the
-    *         given directory isn't in the user
-    *         temp dir location
-    */
-  def deleteTmpDir(dir: File): Boolean = {
-    val isTemp = dir.getPath startsWith Properties.tmpDir
-    if (!isTemp) {
-      logger.warn(
-        s"Directory $dir is not in the system temp dir location! You most likely didn't mean to delete this directory.")
-      false
-    } else if (!dir.isDirectory) {
-      dir.delete()
-    } else {
-      dir.listFiles().foreach(deleteTmpDir)
-      dir.delete()
-    }
-  }
-
   /**
     * Awaits non-blockingly until the provided clients are connected
     */
@@ -831,7 +812,7 @@ trait BitcoindRpcTestUtil extends BitcoinSLogger {
       implicit executionContext: ExecutionContext): Future[Unit] = {
     val stopsF = List(client1, client2).map { client =>
       client.stop().map { _ =>
-        deleteTmpDir(client.getDaemon.datadir)
+        FileUtil.deleteTmpDir(client.getDaemon.datadir)
       }
     }
     Future.sequence(stopsF).map(_ => ())

testkit/src/main/scala/org/bitcoins/testkit/util/FileUtil.scala

@@ -1,9 +1,41 @@
 package org.bitcoins.testkit.util
 
-object FileUtil {
+import java.io.File
+import java.nio.file.Path
+
+import org.bitcoins.core.util.BitcoinSLogger
+
+import scala.util.Properties
+
+object FileUtil extends BitcoinSLogger {
 
   /** Returns a `BufferedSource` for any file on the classpath */
   def getFileAsSource(fileName: String): scala.io.BufferedSource = {
     scala.io.Source.fromURL(getClass.getResource(s"/$fileName"))
   }
+
+  /**
+    * Deletes the given temporary directory
+    *
+    * @throws IllegalArgumentException if the
+    *         given directory isn't in the user
+    *         temp dir location
+    */
+  def deleteTmpDir(dir: File): Boolean = {
+    val isTemp = dir.getPath startsWith Properties.tmpDir
+    if (!isTemp) {
+      logger.warn(
+        s"Directory $dir is not in the system temp dir location! You most likely didn't mean to delete this directory.")
+      false
+    } else if (!dir.isDirectory) {
+      dir.delete()
+    } else {
+      dir.listFiles().foreach(deleteTmpDir)
+      dir.delete()
+    }
+  }
+
+  def deleteTmpDir(path: Path): Boolean = {
+    deleteTmpDir(path.toFile)
+  }
 }

testkit/src/main/scala/org/bitcoins/testkit/util/ScalaTestUtil.scala

@@ -0,0 +1,21 @@
+package org.bitcoins.testkit.util
+
+import org.scalatest.{Assertion, Assertions}
+
+import scala.concurrent.{ExecutionContext, Future}
+
+/** Helper methods for working with the scalatest testing framewrok
+  * @see [[http://www.scalatest.org/user_guide/using_assertions scalatest documentation]]
+  * */
+object ScalaTestUtil {
+
+  def toAssertF(vecFut: Vector[Future[Assertion]])(
+      implicit ec: ExecutionContext): Future[Assertion] = {
+    val futVec = Future.sequence(vecFut)
+    futVec.map(_.foldLeft(Assertions.succeed) {
+      case (_, next) =>
+        next
+    })
+  }
+
+}

testkit/src/test/scala/org/bitcoins/testkit/util/ScalaTestUtilTest.scala

@@ -0,0 +1,37 @@
+package org.bitcoins.testkit.util
+
+import org.scalatest.Assertion
+import org.scalatest.exceptions.TestFailedException
+
+import scala.concurrent.Future
+
+class ScalaTestUtilTest extends BitcoinSUnitTest {
+  implicit val ec: scala.concurrent.ExecutionContext =
+    scala.concurrent.ExecutionContext.global
+
+  behavior of "ScalaTestUtilTest"
+
+  def t = assert(true)
+
+  def f = assert(false)
+
+  def futureFail = Future {
+    //sleep for awhile and then eventually fail
+    Thread.sleep(1000)
+    f
+  }
+
+  it must "evaluate a Vector[Future[Assertions]] correctly" in {
+    val vec1: Vector[Future[Assertion]] =
+      Vector(Future.successful(t), Future.successful(t))
+
+    ScalaTestUtil.toAssertF(vec1)
+
+    try {
+      ScalaTestUtil.toAssertF(Vector(futureFail))
+    } catch {
+      case _: TestFailedException => succeed
+    }
+
+  }
+}