From 6330e76daf6c924d5e5b0ac031ed5ceaf88fbb2d Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Mon, 30 Nov 2020 18:10:21 +0100 Subject: [PATCH] [CI] Exclude tests that call external APIs (#1622) These tests have been flaky because external APIs tend to be down or throttle our calls. and we don't want our test suite to fail because of that. In practice it's enough if developers run these tests locally once in a while. --- .github/workflows/main.yml | 4 +++- .../test/scala/fr/acinq/eclair/TestConstants.scala | 12 +++++++++++- .../eclair/blockchain/fee/BitgoFeeProviderSpec.scala | 6 +++--- .../blockchain/fee/EarnDotComFeeProviderSpec.scala | 6 +++--- .../watchdogs/BlockchainWatchdogSpec.scala | 5 +++-- .../blockchain/watchdogs/ExplorerApiSpec.scala | 5 +++-- .../blockchain/watchdogs/HeadersOverDnsSpec.scala | 7 ++++--- 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 550c798468..aff1040054 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,8 +29,10 @@ jobs: - name: Configure OS settings run: echo "fs.file-max = 1024000" | sudo tee -a /etc/sysctl.conf + # NB: we exclude external API tests from the CI, because we don't want our build to fail because a dependency is failing. + # This means we won't automatically catch changes in external APIs, but developers should regularly run the test suite locally so in practice it shouldn't be a problem. - name: Build with Maven - run: mvn compile && mvn scoverage:report + run: mvn compile && mvn scoverage:report -DtagsToExclude=external-api - name: Upload coverage to Codecov uses: codecov/codecov-action@v1 diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala index ebd48b1fea..8fb214bb2e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala @@ -21,7 +21,7 @@ import java.util.UUID import java.util.concurrent.atomic.AtomicLong import com.opentable.db.postgres.embedded.EmbeddedPostgres -import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey} +import fr.acinq.bitcoin.Crypto.PrivateKey import fr.acinq.bitcoin.{Block, ByteVector32, Script} import fr.acinq.eclair.FeatureSupport.Optional import fr.acinq.eclair.Features._ @@ -35,6 +35,7 @@ import fr.acinq.eclair.db.sqlite._ import fr.acinq.eclair.io.{Peer, PeerConnection} import fr.acinq.eclair.router.Router.RouterConf import fr.acinq.eclair.wire.{Color, EncodingType, NodeAddress} +import org.scalatest.Tag import scodec.bits.ByteVector import scala.concurrent.duration._ @@ -132,9 +133,11 @@ object TestConstants { } val pluginParams = new CustomFeaturePlugin { + // @formatter:off override def messageTags: Set[Int] = Set(60003) override def feature: Feature = TestFeature override def name: String = "plugin for testing" + // @formatter:on } object Alice { @@ -337,4 +340,11 @@ object TestConstants { ) } +} + +object TestTags { + + // Tests that call an external API (which may start failing independently of our code). + object ExternalApi extends Tag("external-api") + } \ No newline at end of file diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitgoFeeProviderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitgoFeeProviderSpec.scala index 6b9a6154cb..aac45c4a2b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitgoFeeProviderSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitgoFeeProviderSpec.scala @@ -20,7 +20,7 @@ import akka.actor.ActorSystem import akka.util.Timeout import com.softwaremill.sttp.okhttp.OkHttpFutureBackend import fr.acinq.bitcoin.Block -import fr.acinq.eclair.LongToBtcAmount +import fr.acinq.eclair.{LongToBtcAmount, TestTags} import org.json4s.DefaultFormats import org.scalatest.funsuite.AnyFunSuite @@ -71,7 +71,7 @@ class BitgoFeeProviderSpec extends AnyFunSuite { assert(feerates === ref) } - test("make sure API hasn't changed") { + test("make sure API hasn't changed", TestTags.ExternalApi) { import scala.concurrent.duration._ implicit val system = ActorSystem("test") implicit val ec = system.dispatcher @@ -81,7 +81,7 @@ class BitgoFeeProviderSpec extends AnyFunSuite { assert(Await.result(bitgo.getFeerates, timeout.duration).block_1.toLong > 0) } - test("check that read timeout is enforced") { + test("check that read timeout is enforced", TestTags.ExternalApi) { import scala.concurrent.duration._ implicit val system = ActorSystem("test") implicit val ec = system.dispatcher diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/EarnDotComFeeProviderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/EarnDotComFeeProviderSpec.scala index 097569311a..f0599534af 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/EarnDotComFeeProviderSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/EarnDotComFeeProviderSpec.scala @@ -18,7 +18,7 @@ package fr.acinq.eclair.blockchain.fee import akka.util.Timeout import com.softwaremill.sttp.okhttp.OkHttpFutureBackend -import fr.acinq.eclair.LongToBtcAmount +import fr.acinq.eclair.{LongToBtcAmount, TestTags} import grizzled.slf4j.Logging import org.json4s.DefaultFormats import org.scalatest.funsuite.AnyFunSuite @@ -70,7 +70,7 @@ class EarnDotComFeeProviderSpec extends AnyFunSuite with Logging { assert(feerates === ref) } - test("make sure API hasn't changed") { + test("make sure API hasn't changed", TestTags.ExternalApi) { import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.duration._ implicit val sttpBackend = OkHttpFutureBackend() @@ -79,7 +79,7 @@ class EarnDotComFeeProviderSpec extends AnyFunSuite with Logging { logger.info("earn.com livenet fees: " + Await.result(provider.getFeerates, 10 seconds)) } - test("check that read timeout is enforced") { + test("check that read timeout is enforced", TestTags.ExternalApi) { import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.duration._ implicit val sttpBackend = OkHttpFutureBackend() diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/BlockchainWatchdogSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/BlockchainWatchdogSpec.scala index 4176cd1800..70186ebb8c 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/BlockchainWatchdogSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/BlockchainWatchdogSpec.scala @@ -20,6 +20,7 @@ import akka.actor.testkit.typed.scaladsl.{ScalaTestWithActorTestKit, TestProbe} import akka.actor.typed.eventstream.EventStream import com.typesafe.config.ConfigFactory import fr.acinq.bitcoin.Block +import fr.acinq.eclair.TestTags import fr.acinq.eclair.blockchain.watchdogs.BlockchainWatchdog.{DangerousBlocksSkew, WrappedCurrentBlockCount} import org.scalatest.funsuite.AnyFunSuiteLike @@ -27,7 +28,7 @@ import scala.concurrent.duration.DurationInt class BlockchainWatchdogSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("application")) with AnyFunSuiteLike { - test("fetch block headers from four sources on mainnet") { + test("fetch block headers from four sources on mainnet", TestTags.ExternalApi) { val eventListener = TestProbe[DangerousBlocksSkew]() system.eventStream ! EventStream.Subscribe(eventListener.ref) val watchdog = testKit.spawn(BlockchainWatchdog(Block.LivenetGenesisBlock.hash, 1 second)) @@ -44,7 +45,7 @@ class BlockchainWatchdogSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa testKit.stop(watchdog) } - test("fetch block headers from three sources on testnet") { + test("fetch block headers from three sources on testnet", TestTags.ExternalApi) { val eventListener = TestProbe[DangerousBlocksSkew]() system.eventStream ! EventStream.Subscribe(eventListener.ref) val watchdog = testKit.spawn(BlockchainWatchdog(Block.TestnetGenesisBlock.hash, 1 second)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/ExplorerApiSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/ExplorerApiSpec.scala index 9bddaf5f43..2b1002cc78 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/ExplorerApiSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/ExplorerApiSpec.scala @@ -19,6 +19,7 @@ package fr.acinq.eclair.blockchain.watchdogs import akka.actor.testkit.typed.scaladsl.ScalaTestWithActorTestKit import com.typesafe.config.ConfigFactory import fr.acinq.bitcoin.Block +import fr.acinq.eclair.TestTags import fr.acinq.eclair.blockchain.watchdogs.BlockchainWatchdog.LatestHeaders import fr.acinq.eclair.blockchain.watchdogs.ExplorerApi.{BlockcypherExplorer, BlockstreamExplorer, CheckLatestHeaders, MempoolSpaceExplorer} import org.scalatest.funsuite.AnyFunSuiteLike @@ -27,7 +28,7 @@ class ExplorerApiSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("appl val explorers = Seq(BlockcypherExplorer(), BlockstreamExplorer(), MempoolSpaceExplorer()) - test("fetch latest block headers") { + test("fetch latest block headers", TestTags.ExternalApi) { for (explorer <- explorers) { val api = testKit.spawn(ExplorerApi(Block.LivenetGenesisBlock.hash, 630450, explorer)) val sender = testKit.createTestProbe[LatestHeaders]() @@ -40,7 +41,7 @@ class ExplorerApiSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("appl } } - test("fetch future block headers") { + test("fetch future block headers", TestTags.ExternalApi) { for (explorer <- explorers) { val api = testKit.spawn(ExplorerApi(Block.LivenetGenesisBlock.hash, 60000000, explorer)) val sender = testKit.createTestProbe[LatestHeaders]() diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDnsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDnsSpec.scala index b546f8ee69..12ab8a670d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDnsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/watchdogs/HeadersOverDnsSpec.scala @@ -19,6 +19,7 @@ package fr.acinq.eclair.blockchain.watchdogs import akka.actor.testkit.typed.scaladsl.ScalaTestWithActorTestKit import com.typesafe.config.ConfigFactory import fr.acinq.bitcoin.{Block, BlockHeader} +import fr.acinq.eclair.TestTags import fr.acinq.eclair.blockchain.watchdogs.BlockchainWatchdog.{BlockHeaderAt, LatestHeaders} import fr.acinq.eclair.blockchain.watchdogs.HeadersOverDns.CheckLatestHeaders import org.scalatest.funsuite.AnyFunSuiteLike @@ -28,7 +29,7 @@ import scala.concurrent.duration.DurationInt class HeadersOverDnsSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("application")) with AnyFunSuiteLike { - test("fetch latest block headers") { + test("fetch latest block headers", TestTags.ExternalApi) { val headersOverDns = testKit.spawn(HeadersOverDns(Block.LivenetGenesisBlock.hash, 630450)) val sender = testKit.createTestProbe[LatestHeaders]() headersOverDns ! CheckLatestHeaders(sender.ref) @@ -47,14 +48,14 @@ class HeadersOverDnsSpec extends ScalaTestWithActorTestKit(ConfigFactory.load("a sender.expectMessage(LatestHeaders(630450, expectedHeaders, HeadersOverDns.Source)) } - test("fetch future block headers") { + test("fetch future block headers", TestTags.ExternalApi) { val headersOverDns = testKit.spawn(HeadersOverDns(Block.LivenetGenesisBlock.hash, 60000000)) val sender = testKit.createTestProbe[LatestHeaders]() headersOverDns ! CheckLatestHeaders(sender.ref) sender.expectMessage(LatestHeaders(60000000, Set.empty, HeadersOverDns.Source)) } - test("ignore testnet requests") { + test("ignore testnet requests", TestTags.ExternalApi) { val headersOverDns = testKit.spawn(HeadersOverDns(Block.TestnetGenesisBlock.hash, 500000)) val sender = testKit.createTestProbe[LatestHeaders]() headersOverDns ! CheckLatestHeaders(sender.ref)