Update spacemesh API #189
Update spacemesh API #189
Conversation
|
@poszu the unit tests on windows are very unstable. I executed them 4 times and got different errors every time:
I reverted the last merge in this PR, so that we can get a green build again. |
I am aware of these issues with Mac and Windows. Unfortunately, the tests were written with some timing assumptions in mind and:
This is caused by asynchronous round teardown. I plan to fix it in #181.
This is caused by generating empty proofs (0 leaves) - it happens on Mac as it is slow and the round is ended the moment it starts. I don't have a clear idea of how to fix it. Perhaps an empty proof is a valid one and it should be broadcasted empty instead of trying to build leaves and nodes?
It's tracked in #185.
These issues are not directly related to the PR you revert. Reverting doesn't fix the problem, it will persist, perhaps a little less often though. I'm against reverting it. I'd rather stop running tests on mac and windows as they are not the platform that Poets will run on. There's no point in supporting them. |
|
Why is it needed to update |
That might very well be the case but at the moment
I'm against this. Poet is a dependency (as a library not just as a service) of go-spacemesh. go-spacemesh needs to support all 3 platforms, so poet should also be tested for all three. We can maybe skip some of the tests on mac and windows if they are only needed for the service but not for the functionality that go-spacemesh uses. I'd rather stop running tests in parallel if this is unstable until the issues listed above (which seem to be already tracked) are resolved. After that they can be enabled again. The PR that introduced the parallel tests was just unlucky to have a single green build, you can try re-running the CI on the PR and it will fail more often then pass - imo the PR should not have been merged at the current state.
I initially thought it is required to update the API to |
Again, the failures are not caused by running tests in parallel. Tests are flaky nevertheless and the build will continue to fail even after it's reverted. Running tests in parallel is not unstable. The tests are unstable in general.
IMHO, the Poet should not be a dependency of go-spacemesh as a whole or even at all. It's used in two places only:
|
Yes, but running them in parallel causes them more often to fail. The build is red, reverting the PR makes the build green. Shall we now not merge any PRs until the issues are resolved?
This is not the case yet; poet is used on all 3 platforms at the moment. Removing the tests is better than leaving them and just making it less likely that they randomly fail? |
IMO it was luck. |
Update poet with the latest spacemesh API