0xPolygonMiden / miden-base

Core components of the Polygon Miden rollup
MIT License
61 stars 32 forks source link

Refactor mock framework and tests #700

Open bobbinth opened 1 month ago

bobbinth commented 1 month ago

The way we run tests and mock various objects for testing purposes is far from ideal. There are quite a few issues now in this repo related to tests/mock objects and rather than dealing with them one by one, I think it would make sense to make a comprehensive refactoring.

The relevant issues are:

Other relevant issues might be (though, not sure if we should tackle them yet):

Also, there is a relevant discussion in https://github.com/0xPolygonMiden/compiler/discussions/168.

It is not yet clear to me what exactly is the best approach to refactor tests/mocks. It may be that we need to improve the miden-mock crate somehow, or maybe get rid of it entirely and move relevant mocks to different crates.

igamigo commented 4 weeks ago

Some comments/questions about the overall goals:

hackaugusto commented 4 weeks ago

Is the mock executable used anywhere? I've investigated and it does not seem to be, but it's hard to tell since something outside of the repo could be relying on it.

AFAIK it is currently not used. The idea was to use it to generate a valid set of accounts, since these require PoW, and then just load them on the tests (specially because in the beginning we didn't have the reduction to the PoW behind the testing flag, so all tests creating accounts would timeout). Another alternative is to do the PoW and save the seed in the source code.

igamigo commented 4 weeks ago

After doing some basic tests, I agree that removing the crate is the better solution and I'm currently making those changes.

bobbinth commented 4 weeks ago

What do we want from the mocking code in relation to the use-cases? For example, the discussion on the compiler repo paints a picture that would be useful for external users such as contract developers.

The primary goal would be to make testing the code in miden-base easier (right now creating and updating tests has become a significant burden). But I think we should keep the external usage in mind as it will simplify our life down the road.

It looks like some of these mentioned items are more functional (like not compiling on no_std contexts), some more related to features, and some others related to code structure. I'm thinking of fixing all issues that are more relevant, making sure there is no duplicate code where possible, and proceeding with the refactor in order to add the missing features. I like the proposal in the compiler repo so I think I would use some of the missing tests (Create 2 transaction tests that consume and output notes #446, Implement test for producing and proving of transaction for new account #635) to guide the implementation of these features and API.

Sounds good!