From 4a80426feec41f070973413fbeddf8aa19704527 Mon Sep 17 00:00:00 2001 From: pm47 Date: Mon, 16 May 2022 16:38:00 +0200 Subject: [PATCH 1/4] make state specs run sequentially --- .../channel/states/ChannelStateTestsHelperMethods.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala index d7364eba51..cbffd9cc3a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala @@ -20,9 +20,9 @@ import akka.actor.typed.scaladsl.adapter.actorRefAdapter import akka.actor.{ActorContext, ActorRef} import akka.testkit.{TestFSMRef, TestKitBase, TestProbe} import com.softwaremill.quicklens.ModifyPimp +import fr.acinq.bitcoin.ScriptFlags import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey import fr.acinq.bitcoin.scalacompat.{ByteVector32, Crypto, SatoshiLong, Transaction} -import fr.acinq.bitcoin.ScriptFlags import fr.acinq.eclair.TestConstants.{Alice, Bob} import fr.acinq.eclair._ import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ @@ -39,7 +39,7 @@ import fr.acinq.eclair.router.Router.ChannelHop import fr.acinq.eclair.transactions.Transactions import fr.acinq.eclair.transactions.Transactions._ import fr.acinq.eclair.wire.protocol._ -import org.scalatest.{FixtureTestSuite, ParallelTestExecution} +import org.scalatest.FixtureTestSuite import java.util.UUID import scala.concurrent.duration._ @@ -47,7 +47,7 @@ import scala.concurrent.duration._ /** * Created by PM on 23/08/2016. */ -trait ChannelStateTestsBase extends ChannelStateTestsHelperMethods with FixtureTestSuite with ParallelTestExecution { +trait ChannelStateTestsBase extends ChannelStateTestsHelperMethods with FixtureTestSuite { implicit class ChannelWithTestFeeConf(a: TestFSMRef[ChannelState, ChannelData, Channel]) { // @formatter:off From 228addfe9c915a6ec2d3f769e4d55b00b55e974b Mon Sep 17 00:00:00 2001 From: pm47 Date: Mon, 16 May 2022 16:38:27 +0200 Subject: [PATCH 2/4] split large test files --- .../eclair/channel/states/e/NormalStateSpec.scala | 11 +++++++++-- .../eclair/channel/states/h/ClosingStateSpec.scala | 14 +++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index 59981ba15b..778b896601 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -52,7 +52,7 @@ import scala.concurrent.duration._ * Created by PM on 05/07/2016. */ -class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { +abstract class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { type FixtureParam = SetupFixture @@ -68,8 +68,11 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with withFixture(test.toNoArgTest(setup)) } } +} + +class Chunk1NormalStateSpec extends NormalStateSpec { - test("recv CMD_ADD_HTLC (empty origin)") { f => + test("recv CMD_ADD_HTLC (empty origin)") { f => import f._ val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] val sender = TestProbe() @@ -1916,6 +1919,10 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with assert(fail.reason.length === Sphinx.FailurePacket.PacketLength) } +} + +class Chunk2NormalStateSpec extends NormalStateSpec { + private def testCmdUpdateFee(f: FixtureParam): Unit = { import f._ val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala index 6fcaca1652..4c92f12a90 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala @@ -17,9 +17,9 @@ package fr.acinq.eclair.channel.states.h import akka.testkit.{TestFSMRef, TestProbe} +import fr.acinq.bitcoin.ScriptFlags import fr.acinq.bitcoin.scalacompat.Crypto.PrivateKey import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, OutPoint, SatoshiLong, Script, Transaction, TxIn, TxOut} -import fr.acinq.bitcoin.ScriptFlags import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ import fr.acinq.eclair.blockchain.fee.{FeeratePerKw, FeeratesPerKw} import fr.acinq.eclair.channel._ @@ -29,7 +29,7 @@ import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishRepla import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.payment._ import fr.acinq.eclair.payment.relay.Relayer._ -import fr.acinq.eclair.transactions.Transactions.{AnchorOutputsCommitmentFormat, DefaultCommitmentFormat, HtlcSuccessTx, HtlcTimeoutTx, ZeroFeeHtlcTxAnchorOutputsCommitmentFormat} +import fr.acinq.eclair.transactions.Transactions._ import fr.acinq.eclair.transactions.{Scripts, Transactions} import fr.acinq.eclair.wire.protocol._ import fr.acinq.eclair.{BlockHeight, CltvExpiry, CltvExpiryDelta, Features, MilliSatoshiLong, TestConstants, TestKitBaseClass, TimestampSecond, randomBytes32, randomKey} @@ -43,7 +43,7 @@ import scala.concurrent.duration._ * Created by PM on 05/07/2016. */ -class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { +abstract class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { case class FixtureParam(alice: TestFSMRef[ChannelState, ChannelData, Channel], bob: TestFSMRef[ChannelState, ChannelData, Channel], alice2bob: TestProbe, bob2alice: TestProbe, alice2blockchain: TestProbe, bob2blockchain: TestProbe, alice2relayer: TestProbe, bob2relayer: TestProbe, channelUpdateListener: TestProbe, txListener: TestProbe, bobCommitTxs: List[CommitTxAndRemoteSig]) @@ -120,6 +120,10 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with } } +} + +class Chunk1ClosingStateSpec extends ClosingStateSpec { + test("recv BITCOIN_FUNDING_PUBLISH_FAILED", Tag("funding_unconfirmed")) { f => import f._ val sender = TestProbe() @@ -999,6 +1003,10 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with claimHtlcTimeoutTxs.foreach(claimHtlcTimeout => assert(alice2blockchain.expectMsgType[WatchOutputSpent].outputIndex === claimHtlcTimeout.input.outPoint.index)) } +} + +class Chunk2ClosingStateSpec extends ClosingStateSpec { + private def testFutureRemoteCommitTxConfirmed(f: FixtureParam, channelFeatures: ChannelFeatures): Transaction = { import f._ val oldStateData = alice.stateData From c272f544654438c6286635ce1c93543a812a4e92 Mon Sep 17 00:00:00 2001 From: pm47 Date: Mon, 16 May 2022 18:09:02 +0200 Subject: [PATCH 3/4] Revert "split large test files" This reverts commit 228addfe9c915a6ec2d3f769e4d55b00b55e974b. --- .../eclair/channel/states/e/NormalStateSpec.scala | 11 ++--------- .../eclair/channel/states/h/ClosingStateSpec.scala | 14 +++----------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index 778b896601..59981ba15b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -52,7 +52,7 @@ import scala.concurrent.duration._ * Created by PM on 05/07/2016. */ -abstract class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { +class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { type FixtureParam = SetupFixture @@ -68,11 +68,8 @@ abstract class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteL withFixture(test.toNoArgTest(setup)) } } -} - -class Chunk1NormalStateSpec extends NormalStateSpec { - test("recv CMD_ADD_HTLC (empty origin)") { f => + test("recv CMD_ADD_HTLC (empty origin)") { f => import f._ val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] val sender = TestProbe() @@ -1919,10 +1916,6 @@ class Chunk1NormalStateSpec extends NormalStateSpec { assert(fail.reason.length === Sphinx.FailurePacket.PacketLength) } -} - -class Chunk2NormalStateSpec extends NormalStateSpec { - private def testCmdUpdateFee(f: FixtureParam): Unit = { import f._ val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala index 4c92f12a90..6fcaca1652 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala @@ -17,9 +17,9 @@ package fr.acinq.eclair.channel.states.h import akka.testkit.{TestFSMRef, TestProbe} -import fr.acinq.bitcoin.ScriptFlags import fr.acinq.bitcoin.scalacompat.Crypto.PrivateKey import fr.acinq.bitcoin.scalacompat.{ByteVector32, ByteVector64, Crypto, OutPoint, SatoshiLong, Script, Transaction, TxIn, TxOut} +import fr.acinq.bitcoin.ScriptFlags import fr.acinq.eclair.blockchain.bitcoind.ZmqWatcher._ import fr.acinq.eclair.blockchain.fee.{FeeratePerKw, FeeratesPerKw} import fr.acinq.eclair.channel._ @@ -29,7 +29,7 @@ import fr.acinq.eclair.channel.publish.TxPublisher.{PublishFinalTx, PublishRepla import fr.acinq.eclair.channel.states.{ChannelStateTestsBase, ChannelStateTestsTags} import fr.acinq.eclair.payment._ import fr.acinq.eclair.payment.relay.Relayer._ -import fr.acinq.eclair.transactions.Transactions._ +import fr.acinq.eclair.transactions.Transactions.{AnchorOutputsCommitmentFormat, DefaultCommitmentFormat, HtlcSuccessTx, HtlcTimeoutTx, ZeroFeeHtlcTxAnchorOutputsCommitmentFormat} import fr.acinq.eclair.transactions.{Scripts, Transactions} import fr.acinq.eclair.wire.protocol._ import fr.acinq.eclair.{BlockHeight, CltvExpiry, CltvExpiryDelta, Features, MilliSatoshiLong, TestConstants, TestKitBaseClass, TimestampSecond, randomBytes32, randomKey} @@ -43,7 +43,7 @@ import scala.concurrent.duration._ * Created by PM on 05/07/2016. */ -abstract class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { +class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with ChannelStateTestsBase { case class FixtureParam(alice: TestFSMRef[ChannelState, ChannelData, Channel], bob: TestFSMRef[ChannelState, ChannelData, Channel], alice2bob: TestProbe, bob2alice: TestProbe, alice2blockchain: TestProbe, bob2blockchain: TestProbe, alice2relayer: TestProbe, bob2relayer: TestProbe, channelUpdateListener: TestProbe, txListener: TestProbe, bobCommitTxs: List[CommitTxAndRemoteSig]) @@ -120,10 +120,6 @@ abstract class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuite } } -} - -class Chunk1ClosingStateSpec extends ClosingStateSpec { - test("recv BITCOIN_FUNDING_PUBLISH_FAILED", Tag("funding_unconfirmed")) { f => import f._ val sender = TestProbe() @@ -1003,10 +999,6 @@ class Chunk1ClosingStateSpec extends ClosingStateSpec { claimHtlcTimeoutTxs.foreach(claimHtlcTimeout => assert(alice2blockchain.expectMsgType[WatchOutputSpent].outputIndex === claimHtlcTimeout.input.outPoint.index)) } -} - -class Chunk2ClosingStateSpec extends ClosingStateSpec { - private def testFutureRemoteCommitTxConfirmed(f: FixtureParam, channelFeatures: ChannelFeatures): Transaction = { import f._ val oldStateData = alice.stateData From ca7a3ba8175ed7568540e1fd82ae4eb146eabf86 Mon Sep 17 00:00:00 2001 From: pm47 Date: Mon, 16 May 2022 18:27:48 +0200 Subject: [PATCH 4/4] add guidelines on testing in CONTRIBUTING.md --- CONTRIBUTING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 70687fd2c7..96aa2fb1d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,6 +147,14 @@ If your contribution is adding a new dependency, please detail: Contributions that add new dependencies may take longer to approve because a detailed audit of the dependency may be required. +### Testing + +Your code should be tested. We use ScalaTest as a testing framework. + +ScalaTest's approach is to parallelize on test suites rather than individual tests, therefore it is +recommended to keep the execution time of each test suite under one minute and split tests across +multiple smaller suites if needed. + ### IntelliJ Tips If you're using [IntelliJ](https://www.jetbrains.com/idea/), here are some useful commands: