Better Test Framework / Making `gadget_tester` Public

ZK-Garage / plonk

A pure Rust PLONK implementation using arkworks as a backend.

https://discord.gg/XWJdhVf37F

Mozilla Public License 2.0

295 stars 76 forks source link

Better Test Framework / Making `gadget_tester` Public #51

Closed tsunrise closed 2 years ago

tsunrise commented 2 years ago

I really like the gadget_tester in ark-plonk, which makes writing end-to-end tests much easier, but this gadget is pub(crate) so user cannot access it...

I believe many users will end up writing their own gadgets (for gadgets, I mean any function that includes StandardComposer as a parameter). Can we make the following public:

gadget_tester
batch_test!

If there is any reason that prevents doing so, shall we have a good testing framework? Writing end-to-end tests for a gadget from scratch is a bit verbose.

Thanks!

bhgomes commented 2 years ago

~~In some cases, lots of StandardComposer internals will also have to be exposed for crates that write their own custom gates/gadgets. We should discuss how to do this safely.~~

After discussion with @tsunrise, we should just focus on the testing suite first; safe access to StandardComposer isn't needed yet.

bhgomes commented 2 years ago

I propose the following design:

Add a feature test for testing suite (NB: This is not the same as cfg(test) in Rust which is a special profile which is run during cargo test. The test feature is for library-exported content).
Move testing-related code into a new module test which is feature-gated with the test feature.
Expose at least gadget_tester (with a better name) and batch_test!. We should revisit the interfaces of these to make them as usable/general as possible.

CPerezz commented 2 years ago

I agree with @bhgomes.

I see two ways here.

The feature (if we plan to add one or two functions as testing helpers) as much.
A testing lib. With more complicated primitives.

bhgomes commented 2 years ago

Why don't we start with a small feature-gated library in the main codebase and if we have more demand for complex testing we add a workspace crate with more stuff.

tsunrise commented 2 years ago

If no one has started implementing it yet, I can do it. Should I start drafting a PR?

CPerezz commented 2 years ago

If no one has started implementing it yet, I can do it. Should I start drafting a PR?

Of course!! Feel free to go for it!!

CPerezz commented 2 years ago

@tsunrise are you still planning to do this?

tsunrise commented 2 years ago

Oops, I thought this issue was closed yesterday. No, because I don't see any immediate need for that. I think check_satisfied function is enough.

close this issue for now, although I'm happy to work on this if someone wants it 😃