8000 Improve caching for Community Integration Tests against Pro by dominikschubert · Pull Request #9114 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

Improve caching for Community Integration Tests against Pro #9114

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Sep 12, 2023
Merged

Improve caching for Community Integration Tests against Pro #9114

merged 14 commits into from
Sep 12, 2023

Conversation

dominikschubert
Copy link
Member
@dominikschubert dominikschubert commented Sep 12, 2023 •
edited
Loading

Motivation

Currently the Community Integration Tests against Pro are the slowest part of our community test suite on PRs.
In this PR we attempt to reduce the runtime of these tests to get them to a point where we don't have to wait an additional 20+ minutes until the pro tests against community are done for a green pipeline.

The initial commit will currently never find a cache hit since we don't execute the action on master. Additionally the cache key seems to be calculated incorrectly by considering too many files for caching and never actually restored the venv/lib cache, even in a single PR with multiple runs.

Successful caching of the built lambda common packages should already save ~8 minutes.

Changes

  • Stricter matching/hash calculation
  • Add a restore key for the venv/lib caching to match cache from master.
    • this should work for the venv but for the libs it will only be able to cache half since we're currently always splitting the tests into two, so not all installers might have been executed.
  • Execute action on master again to populate cache for PRs to use
    • We don't really want to do this on every commit, so this will only be executed when a potential cache miss would happen (matching on the same set of files)
    • This is supplemented with a daily scheduled run at 4 AM UTC to also cover potential changes in -ext, at least on a daily interval. Ideally this will be replaced with a trigger from -ext on successful runs that will manually start the workflow in community.
  • Set DISABLE_BOTO_RETRIES=1 for test execution

Outcome

The proper caching of the venv + libs wasn't as impactful as I had initially hoped. It still reduced the total run time by a few (~4) minutes. In combination with the saved 8 minutes ( from the lambda common cache) of the initial test run, a PR's first run of Community Integration Tests against Pro should be about ~12 min faster now.

@dominikschubert dominikschubert self-assigned this Sep 12, 2023
@dominikschubert dominikschubert added the semver: patch Non-breaking changes which can be included in patch releases label Sep 12, 2023
@github-actions
Copy link
github-actions bot commented Sep 12, 2023 •
edited
Loading

LocalStack Community integration with Pro

       2 files         2 suites   1h 9m 10s ⏱️
2 181 tests 1 696 ✔️ 485 💤 0 ❌
2 182 runs  1 696 ✔️ 486 💤 0 ❌

Results for commit 6f92a5d.

♻️ This comment has been updated with latest results.

@coveralls
Copy link
coveralls commented Sep 12, 2023 •
edited
Loading

Coverage Status

coverage: 79.758% (+0.004%) from 79.754% when pulling 6f92a5d on fix-pro-integration-caching into a3eb98c on master.

@dominikschubert dominikschubert marked this pull request as ready for review September 12, 2023 12:59
alexrashed
alexrashed requested changes Sep 12, 2023
View reviewed changes
Copy link
Member
@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for jumping on this! I think we have to adjust the restore key a bit, but then this should greatly speed up the community tests in our PRs! 🚀 ⌛

localstack/aws/connect.py
@@ -49,7 +49,6 @@ def my_patch(fn, self, **kwargs):
patched_kwargs = {
**kwargs,
"WaiterConfig": {
# TODO: make these configurable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't seem to be related, but if it's on purpose, it's okay to sneak this one in ;)

@dominikschubert dominikschubert merged commit 7974a9c into master Sep 12, 2023
@dominikschubert dominikschubert deleted the fix-pro-integration-caching branch September 12, 2023 15:05
Sign up for free 75D7 to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed alexrashed approved these changes

@thrau thrau Awaiting requested review from thrau

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

Assignees

@dominikschubert dominikschubert

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dominikschubert @coveralls @alexrashed
0