jTendermint/MerkleTree

6/23 Failing Tests

Closed this issue · 8 comments

I did fork your project some time ago and just wanted to upgrade my fork to your upstream master, but see the following 6 out of 23 tests failing:

  • MerkleTreeTest.testGetAndRemove
  • MerkleTreeTest.testGetRootHash
  • MerkleTreeTest.testToPrettyString
  • ByteableLongTest.testEqualsHashCodeToString
  • ByteableStringTest.testByteAbleString
  • ByteableStringTest.testEqualsHashCodeToString

Am I missing something? Any plans to address this?

srmo commented

Oh. It’s been quite some time since I looked at this repo.
Give us a bit to look into it.

srmo commented

PS: we should definitely start to put this under circleCi

srmo commented

@tglaeser ok, I just had a more deep look at the state of this repo and it's a mess. We will bring this up to speed. Will take a bit of refactoring and package renaming and implementing methods that somehow got lost. Will take a few days :)

Thanks @srmo for looking into this.

Considering that repositories MerkleTree, crypto, and jabci are related, I was thinking it would make sense to move them all under a Gradle multi-project build. If you agree, I could help to get this setup. This should also simplify CI and code quality reporting.

srmo commented

@tglaeser at this point, I don't see a dependency between MerkleTree and jabci.
At least we don't use it as such. I don't have much experience with gradle so I fail to understand how we then would include jABCI dependency in our external maven project. I guess it doesn't matter as we just reference the release artifact from central?

@srmo you are right, MerkleTree only depends on crypto, but when you write an ABCI application in Java then you will have dependencies on MerkleTree and jabci. As an example, see here how this could be described with Gradle. You find a fork of those three repositories under modules.

srmo commented

@tglaeser tests have been quickly fixed and the package structure has been put into a more clean state. Closing this issue.

Thanks @srmo, very much appreciated.