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

services/horizon: Improve sponsorship integration tests' running time & flexibility. #3090

Merged
merged 13 commits into from
Oct 7, 2020

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Oct 2, 2020

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into two PRs
    otherwise).
  • This PR's title starts with name of package that is most changed in the PR, ex.
    services/friendbot, or all or doc if the changes are broad or impact many
    packages.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or fixes.
  • I've updated any docs (developer docs, .md
    files, etc... affected by this change). Take a look in the docs folder for a given service,
    like this one.

Release planning

  • I've updated the relevant CHANGELOG (here for Horizon) if
    needed with deprecations, added features, breaking changes, and DB schema changes.
  • I've decided if this PR requires a new major/minor version according to
    semver, or if it's mainly a patch change. The PR is targeted at the next
    release branch if it's not a patch change.

What

Refactors the sponsorship (CAP-33) integration tests:

  • Each test is independently runs as a sub-test of the "TestSponsorships" test. This means only one container up/down!
  • It introduces helper functions commonly used in each tests such as sandwiching sponsored operations, retrieving operations and effects, and confirming the existence of operations via tt.Condition. This shortens the code verbosity and improves readability!
  • itest.MustGetAccount() replaces instances of a manual force-retrieval of an account, and account/keypair management in general is simpler.
  • tt.Eventually is replaced by tt.Condition

From what I can tell, all of the actual testing is still the same. There may be a way to actually combine ALL of these tests into a single test w/ a handful of transactions (akin to what happened in #3033 with predicates), but this is still subject to investigation.

Why

This massively improves both the running time and readability.
It now takes ~1m30s as opposed to ~4m20s previously.

This should close #3075.

Known limitations

n/a

@Shaptic Shaptic requested review from 2opremio and a team October 2, 2020 18:03
@Shaptic Shaptic self-assigned this Oct 2, 2020
@cla-bot cla-bot bot added the cla: yes label Oct 2, 2020
@Shaptic Shaptic changed the title Cap33 test refactor services/horizon: Refactor sponsorship integration tests to improve running time & readability. Oct 2, 2020
@Shaptic Shaptic changed the title services/horizon: Refactor sponsorship integration tests to improve running time & readability. services/horizon: Improve sponsorship integration tests' running time & flexibility. Oct 2, 2020
@Shaptic Shaptic force-pushed the cap33-test-refactor branch 2 times, most recently from 53cc289 to 59e04eb Compare October 2, 2020 18:16
@Shaptic Shaptic force-pushed the cap33-test-refactor branch from 59e04eb to 05e1542 Compare October 7, 2020 19:35
@Shaptic Shaptic merged commit 63e0e3f into stellar:master Oct 7, 2020
@Shaptic Shaptic deleted the cap33-test-refactor branch October 28, 2020 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor sponsorship integration tests
2 participants