mirrors/eclair
1
0
Fork 0
mirror of https://github.com/ACINQ/eclair.git synced 2024-11-19 01:43:22 +01:00
Code Issues Projects Releases Wiki Activity

Run channel state tests sequentially (#2271)

Browse Source 
From scalatest's `ParallelTestExecution` doc:

> ScalaTest's normal approach for running suites of tests in parallel is to run different suites in parallel, but the tests of any one suite sequentially.
> [...]
> To make it easier for users to write tests that run in parallel, this trait runs each test in its own instance of the class. Running each test in its own instance enables tests to use the same instance vars and mutable objects referenced from instance variables without needing to synchronize. Although ScalaTest provides functional approaches to factoring out common test code that can help avoid such issues, running each test in its own instance is an insurance policy that makes running tests in parallel easier and less error prone.

This means that for each single test of the `channel.states` package, we instantiate one actor system, which contains two thread pools. With default settings, that's a minimum of 2*8 threads per individual test.

That's already pretty bad, and with 65cf238 (#2270) we add a factor of 3 on top of that, which makes us go past the OS limits on github CI.

setup | peak thread count | run time
-------|---------------------|----
baseline | 5447 | 5m 44s
sequential | 1927 | 5m 9s (*)

(*) It's actually so bad, that tests run actually faster without parallelization!
This commit is contained in:
Pierre-Marie Padiou 2022-05-17 10:03:49 +02:00 committed by GitHub
parent 10eb9e932f
commit 8e2fc7acd3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CONTRIBUTING.md
View File

@ -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:

6
eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala
View File

@ -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