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

Re-introduce integration tests for the node #2957

Open
hashmap opened this issue Jul 22, 2019 · 2 comments
Open

Re-introduce integration tests for the node #2957

hashmap opened this issue Jul 22, 2019 · 2 comments

Comments

@hashmap
Copy link
Contributor

hashmap commented Jul 22, 2019

In the past the node and the wallet were in the same repo. When the wallet was extracted into a separate repo, integration tests were moved to - https://github.com/mimblewimble/grin-wallet/tree/master/integration
It would be beneficial to provide a simple way to run integration suits as part of node tests too.
There are several possible options (there are some other for sure):

  • Move part of the tests to grin repo. Many tests requires wallet functionality too, we could move only tests which don't require wallet, but it doesn't sound great. We could create some mock wallet, not sure if this effort is justified.
  • Extract test into another repo and add to grin and grin-wallet as submodule. The issue is how to make cargo happy in this case and how to specify grin-wallet dependency in grin and vice versa.
  • Don't touch anything and always run wallet test with local version of grin repo. It adds some time to CI build and UX is poor.
@WhoNeedszZz
Copy link
Contributor

My understanding is that this situation is a typical reference situation for mocking. As long as the tests are kept up-to-date with current functionality on the other end I can't really see an issue with that. It makes sense that they are separated as a wallet requires connecting to a node, but a node doesn't have to connect to a wallet necessarily.

From my perspective, the suggestion of moving tests around is a signal that the tests could be better isolated. If I'm understanding correctly, the expected behavior for those aspects in question are defined so mocking should be suitable. Is this accurate?

@hashmap
Copy link
Contributor Author

hashmap commented Jan 28, 2020

Mocking is fine in unit tests, the point of integration tests is to check how different components work together, not in isolation. You would end up testing your perfectly behaving mocks.

Node does need to connect to wallet to test mining scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants