-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update rsmt2d in IPLD plugin #290
Conversation
Dang, I think you're right, good find!! The CI isn't calling the tests for the plugin. Also, this was my fault for not calling those tests after using the new rsmt2d. 😅 I think the best way to make sure the tests get called is to add them to the testing |
@@ -11,7 +11,7 @@ require ( | |||
github.com/ipfs/go-verifcid v0.0.1 | |||
github.com/lazyledger/nmt v0.3.1 | |||
// rsmt2d is only used in tests: | |||
github.com/lazyledger/rsmt2d v0.1.1-0.20210327010029-ef1d6c54461e | |||
github.com/lazyledger/rsmt2d v0.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Good find! This looks good to me.
Regarding the tests: I think, we should remove the sub-module (in a separate PR), then, I think the tests under p2p/ipld/nodes will also be ran by CI without any further work.
The rationale behind this structure (the plugin being its own go-module) was that we wanted to test the plgin in the ipld-experiments without depending on the full lazyledger-core module. Looking at this now, it seems like this was a somewhat misguided decision as for the actual experiments it should not matter. Later we can think about putting the plugin into it's own repo instead.
* Updated method for running 200 node test * fix & extend * one more... * update hashes * Adapting text of latency extraction method * Script adjustments * Small precision * Update docs/qa/method.md Thanks! Co-authored-by: Lasaro <lasaro@informal.systems> * Update docs/qa/method.md Co-authored-by: Lasaro <lasaro@informal.systems> * Update docs/qa/method.md Co-authored-by: Lasaro <lasaro@informal.systems> --------- Co-authored-by: Lasaro <lasaro@informal.systems> (cherry picked from commit d82bc77) Co-authored-by: Sergio Mena <sergio@informal.systems>
Also, previously plugin used
rsmt2d v0.1.1-0.20210327010029-ef1d6c54461e
which does not haveNewRSGF8Code
func used by latest plugin tests, making the plugin not able to compile, thus not able to run tests. However, the test on CI looks good, this brings the thought that our CI does not run tests in IPLD plugin. Am I missing something or our CI really ignores the IPLD plugin?