Skip to content
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

Refactor mock framework and tests #700

Closed
10 tasks done
bobbinth opened this issue May 22, 2024 · 5 comments
Closed
10 tasks done

Refactor mock framework and tests #700

bobbinth opened this issue May 22, 2024 · 5 comments
Assignees
Labels
tests Improvements to testing
Milestone

Comments

@bobbinth
Copy link
Contributor

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 0xPolygonMiden/compiler#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.

@bobbinth bobbinth added the tests Improvements to testing label May 22, 2024
@bobbinth bobbinth added this to the v0.4 milestone May 22, 2024
@igamigo igamigo moved this to In Progress in User's testnet May 27, 2024
@igamigo
Copy link
Collaborator

igamigo commented May 28, 2024

Some comments/questions about the overall goals:

  • 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.
  • I think I'm leaning more toward having gated functions/mods in the original code rather than keeping and maintaining the mock crate. After reading the issues and past discussions, it seems that the crate dependency graph would be simplified although the main codebase might become a bit more bloated. However this is also tied to the previous question and how we want to present these features to other users as well. I have not had to use the mock crate myself so opinions of people who have used this in the past are welcome (I think @hackaugusto was proposing to remove the crate originally).
  • 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.
  • 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.

@hackaugusto
Copy link
Contributor

hackaugusto commented May 28, 2024

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
Copy link
Collaborator

igamigo commented May 28, 2024

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

@bobbinth
Copy link
Contributor Author

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!

@bobbinth bobbinth modified the milestones: v0.4, v0.5 Jun 24, 2024
@bobbinth bobbinth modified the milestones: v0.5, v0.6 Aug 27, 2024
@Dominik1999 Dominik1999 moved this from In Progress to In Review in User's testnet Sep 12, 2024
@bobbinth bobbinth modified the milestones: v0.6, v0.7 Nov 7, 2024
@igamigo
Copy link
Collaborator

igamigo commented Nov 15, 2024

I think this can be closed, planning on opening an issue with potential followups soon (though some of what I was planning was tackled by Philipp recently)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Improvements to testing
Projects
Status: Done
Development

No branches or pull requests

3 participants