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

Add End-to-End tests for the Miden Node #222

Open
Dominik1999 opened this issue Feb 12, 2024 · 8 comments
Open

Add End-to-End tests for the Miden Node #222

Dominik1999 opened this issue Feb 12, 2024 · 8 comments
Milestone

Comments

@Dominik1999
Copy link
Contributor

Dominik1999 commented Feb 12, 2024

We need to define and run integration tests for the Miden Node.

@phklive
Copy link
Contributor

phklive commented Feb 12, 2024

@hackaugusto
Copy link
Contributor

hackaugusto commented Feb 13, 2024

I think this issue is more about an end-to-end test (E2E) instead of the Rust's integration tests. Here are a few notes that I believe are important for these types of tests:

  1. CI:
    1. The E2E only run after successful unit tests
    2. Logs should be aggressively collected and persisted by the CI
  2. Tests:
    1. Have overridable timeouts for environment setup/teardown and test execution. default timeout for tests of around 30secs
    2. Can skip the teardown on test failure
    3. Must have automatic retries with a resetable test timeout configurable by test and disabled by default
  3. Environment:
    1. Must be containerized

The rationale of the above is:

  1. i. E2E tests are super slow, if the unit tests fail it doesn't make sense to waste CI time on the E2E tests
    ii. Debugging E2E tests from CI can be challenging, and the logs are the only tool we have in some cases (specially when tests fail because the CI machines are not that powerful).
  2. i. When debugging/running a single test, the setup/teardown time will dominate the test runtime, this means these settings should be different. The test timeout is important to detect issues that are outside of the test runner, and to incentivize good test writting.
    ii. When debugging a test, it is useful to keep the environment around to inspect further.
    iii. Flakiness is a given, retries on CI will be needed. Retries should be disabled by default because otherwise super badly written tests can get in, this setting is just a last resort kinda of tool.
  3. i. it is impossible to do environment cleanup properly without it (e.g. double fork creates a daemon and there is no reliable way of caughting it without a cgroup). Using containers also allows for parallel test execution (useful when working on different branches).

Ideally we would also have:

  • The ability to run a new E2E test multiple times when it is introduced, say 5 times. To detect flakiness before merging a PR.
  • Collect some metrics about the tests, to know what is slow/flaky and needs an eye to improve
  • Test should be written with no assumption they have a clean slate. This would require a setup/teardown per test, which is super slow.

With the above said. I think the best approach here is to have a hand written test runner, that will setup the container environment first, and then call the tests (which can be a rust binary).

@phklive
Copy link
Contributor

phklive commented Feb 14, 2024

#232 (comment)

@phklive phklive changed the title Add integration tests for the Miden Node Add End-to-End tests for the Miden Node Feb 15, 2024
@phklive
Copy link
Contributor

phklive commented Mar 1, 2024

@bobbinth would be glad to have your view and input on end-to-end tests that we could perform on the miden node.

Thanks

@phklive phklive moved this to In Progress in Builder's testnet Mar 6, 2024
@Dominik1999
Copy link
Contributor Author

Is there something you need to close this issue? Are you actively working on it?

@phklive
Copy link
Contributor

phklive commented Mar 14, 2024

To be able to close this PR I need this PR to be merged, it has been approved it's a matter of hours before I can continue working on the end-to-end testing: #257

@Dominik1999 Dominik1999 moved this from In Progress to Todo in Builder's testnet Mar 18, 2024
@Dominik1999 Dominik1999 moved this from Todo to In Progress in Builder's testnet Mar 18, 2024
@Dominik1999
Copy link
Contributor Author

What is missing here? Do you need any support?

@Dominik1999 Dominik1999 moved this from In Progress to In Review in Builder's testnet Mar 25, 2024
@Dominik1999 Dominik1999 moved this from In Review to Blocked in Builder's testnet Mar 25, 2024
@Dominik1999 Dominik1999 moved this from Blocked to In Review in Builder's testnet Mar 25, 2024
@bobbinth bobbinth added this to the v0.2 milestone Mar 26, 2024
@Dominik1999 Dominik1999 removed this from the v0.2 milestone Apr 12, 2024
@bobbinth bobbinth added this to the v0.3 milestone Apr 12, 2024
@phklive phklive removed their assignment Apr 19, 2024
@phklive
Copy link
Contributor

phklive commented Apr 19, 2024

More research needs to be done regarding end-to-end testing for the Miden node.

Making sure that they provide value in terms of functionality verification.

@bobbinth bobbinth modified the milestones: v0.3, v0.4 Apr 29, 2024
@bobbinth bobbinth modified the milestones: v0.4, v0.5 Jun 10, 2024
@bobbinth bobbinth modified the milestones: v0.5, v0.6 Jul 22, 2024
@Dominik1999 Dominik1999 moved this to In Progress in User's testnet Aug 22, 2024
@Dominik1999 Dominik1999 removed the status in User's testnet Aug 22, 2024
@bobbinth bobbinth modified the milestones: v0.6, v0.7 Sep 2, 2024
@bobbinth bobbinth modified the milestones: v0.7, v0.8 Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

4 participants