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

sdk-integration-tests: runtime regression #2324

Closed
palango opened this issue Nov 5, 2020 · 1 comment · Fixed by #2417
Closed

sdk-integration-tests: runtime regression #2324

palango opened this issue Nov 5, 2020 · 1 comment · Fixed by #2417
Assignees
Labels
infrastructure 🚧 Tests, CI, and general project infrastructure sdk 🖥 test Tests related issues

Comments

@palango
Copy link
Contributor

palango commented Nov 5, 2020

Recently runtime of the sdk-integration-tests went from 6mins to 12mins. We should investigate if we can recover some/all of those losses.

@andrevmatos noted that #2289 might be the culprit:

A nice and easy way to optimize that by a lot would be to make our integration tests use-case based, instead of testing each function separately
e.g. a happy case test: channel open -> deposit -> transfer -> withdraw -> close -> settle -> unlock
than failure cases for individual steps, possibly making better use of the snapshoting feature of ganache (we already have utils for it)
This way, we could decrease the number of integration tests by a lot, and allow finer-granularity tests to depend on a given previous state (e.g. sneaking in a get channels$ test after channel got opened on the happy case test
Most of the time is spent on setting up and resetting these integration tests
I know “mixing” tests isn’t very good, but since the setup times here are so long, and we already have extensive unit tests, maybe we could consider this refactoring..pretty sure we could get it down to a couple of minutes at most
The subkey integration test is an example of a more extensive test which works very well, and is fast despite testing maybe half of what the other tests does (but here using the subkey)

@palango palango added sdk 🖥 test Tests related issues infrastructure 🚧 Tests, CI, and general project infrastructure labels Nov 5, 2020
@andrevmatos
Copy link
Contributor

#2417 improved a lot on this, integration tests there are taking 7min, but we should still refactor this at some point in the future

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure 🚧 Tests, CI, and general project infrastructure sdk 🖥 test Tests related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants