[ETCM-127] Regular sync integration tests #740
Conversation
biandratti
requested review from
KonradStaniec,
ntallar,
mirkoAlic and
lemastero
|
Commits should be signed using PGP. Take a look at: https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/about-commit-signature-verification BTW I have a problem with pushing to repo after switching to GPG, b/c I clone the project with HTTPS. I had to update remote to have: git remote -v
origin [email protected]:input-output-hk/mantis.git (fetch)
origin [email protected]:input-output-hk/mantis.git (push) |
biandratti
force-pushed
the
ETCM-127-it-regularsync
branch
2 times, most recently
from
ba0ceb2 to
ecfd875
Compare
| discoveryStatus = ServerStatus.NotListening | ||
| ) | ||
|
|
||
| lazy val tempDir = Files.createTempDirectory("temp-regular-sync") |
There was a problem hiding this comment.
Temporary directories in some places are cleared like so:
def withDir(testCode: String => Any): Unit = {
val path = Files.createTempDirectory("testdb").getFileName.toString
try {
testCode(path)
} finally {
val dir = new File(path)
assert(!dir.exists() || dir.delete(), "File deletion failed")
}
}i wonder if we should do the same here?
|
After margin [ETCM-141] scalafmt . This PR can be updated by git merge |
|
|
||
| object ValidatorsExecutorAlwaysSucceed extends ValidatorsExecutorAlwaysSucceed | ||
|
|
||
| class FakePeer(peerName: String, fakePeerCustomConfig: FakePeerCustomConfig) |
There was a problem hiding this comment.
to be hones regular sync FakePeer looks almost the same as FastSyncItSpecUtils.FakePeer (except some regular sync specific clasess) maybe we could have only one FakePeer in some kinda SyncItSpecUtils and in regular sync tests start it by startRegularSync and in fast sync tests start it by startFastSync ?
There was a problem hiding this comment.
Add new abstract class called “CommonFakePeer”
| _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumer + 3) | ||
| } yield { | ||
| assert(peer1.bl.getBestBlock().number == peer2.bl.getBestBlock().number) | ||
| (peer1.bl.getBlockByNumber(blockNumer + 1), peer1.bl.getBlockByNumber(blockNumer + 1)) match { |
There was a problem hiding this comment.
maybe it would be more clear to check assert(peer1.bt.getTotalDifficultyByHash(bestBlock.hash) == peer2.bt.getTotalDifficultyByHash(bestBlock.hash)) ? i.e that peers total difficulties are the same after fork resolution
| _ <- peer2.importBlocksUntil(blockNumer)(IdentityUpdate) | ||
| _ <- peer1.connectToPeers(Set(peer2.node)) | ||
| _ <- peer1.startRegularSync().delayExecution(500.milliseconds) | ||
| _ <- peer2.mineNewBlock()(IdentityUpdate).delayExecution(50.milliseconds) |
There was a problem hiding this comment.
maybe we could mine several blocks (like 10-15) ? (it will check if if all of them are properly queued and handled on syncer par)
There was a problem hiding this comment.
I added the next line, where mine 10 new blocks.
_ <- peer2.mineNewBlocks(100.milliseconds, 10)(IdentityUpdate)
biandratti
force-pushed
the
ETCM-127-it-regularsync
branch
from
841b841 to
a1eddfa
Compare
|
|
||
| import scala.concurrent.duration._ | ||
|
|
||
| class RegularSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfter { |
There was a problem hiding this comment.
can we change BeforeAndAfter to BeforeAndAfterAll and then add :
override def afterAll(): Unit = {
testScheduler.shutdown()
testScheduler.awaitTermination(60.second)
}
? ( we should probably do this also in FastSyncItSpec)
| @@ -68,7 +68,8 @@ val root = { | |||
| libraryDependencies ++= dep | |||
| ) | |||
| .settings(executableScriptName := name.value) | |||
| .settings(inConfig(Integration)(Defaults.testSettings :+ (Test / parallelExecution := false)): _*) | |||
| .settings(inConfig(Integration)(Defaults.testSettings | |||
There was a problem hiding this comment.
Maybe we could add formatting to all subprojects ?
Description