Updates dependencies and tests for Synthetic PoRep support #1331
Conversation
|
My intuition is if its optional, then we need to ensure we have defined behavior for everyplace it could exist, under all the combinations. In this way, we'd want to ensure we have the "Branch coverage". Dependency injecting a proof type to the existing tests seems like an intuitive way to orchestrate multiple types of tests easily. Aayush, Shrenuj, or Mikers should be able to glean more information as to how and where we should do that. |
|
Nice -- the fix here is to rebase on |
cryptonemo
force-pushed
the
synthetic-porep
branch
from
43cf2cb to
08e1c6c
Compare
|
Hrm. How does one get the coverage test to not run out of disk space?! 😅 |
Let’s see re-running works lolol |
Tried it already 😓 |
|
Closing due to #1335 |
Unfortunately, I had a similar version working, but hardcoded the dependencies everywhere (very invasive) and trying to resolve the issues by using the patch mechanism at the top-level is not working. Any eyes/assistance on this is appreciated. tl;dr -- this is broken due to the deps/build system.EDIT: Not yet resolved, sorry was on the wrong branchAnother oddity is that since SyntheticPoRep is a new proof type, but is optional, it wasn't clear how to handle in the tests. In the future, it looks like maybe they should be re-factored to take the proof type as an arg so that we can iterate through several(?) I'm not familiar enough with this repo to know if that's a valid test/use-case.