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

[Dependency Cache] Enable Dependency Cache on CI #13303

Merged
merged 12 commits into from
Jan 17, 2025

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Jan 14, 2025

Project Thread: paaHJt-7CE-p2
Related: #13170 + #13122 + #13288
Depends On: a8c-ci-toolkit#135 -> a8c-ci-toolkit#138
Required By: buildkite-ci#576


Description

This PR enables dependency cache on CI for this project. 💾

The dependency cache mechanism is split between:

  • A weekly scheduled save cache build (see commit and buildkite-ci#576 PR), which targets the below tasks only:
    • Mobile App (WooCommerce:assembleJalapenoDebug)
    • Wear App (WooCommerce-Wear:assembleJalapenoDebug)
    • Lint (WooCommerce:lintJalapenoDebug)
    • Unit Tests (ssembleJalapenoDebugUnitTest assembleDebugUnitTest)
    • Android Tests (assembleJalapenoDebugAndroidTest)
  • A restore cache step that is run on targeted pipeline.yml related jobs, via their corresponding individual scripts:
    • Mobile App (prototype-build.sh)
    • Wear App (prototype-build.sh)
    • Lint (lint.sh)
    • Unit Tests (run-unit-tests.sh)
    • Android tests (run-instrumented-tests.sh)

PS.1: The detekt job isn't targeted because it only takes about 20+ seconds of network request to download all the dependencies needed for this specific job (build + scan), which is about the same time it takes for the dependency cache to actually be restored, which is more or less about 10+ seconds (build). Thus, this optimization is not really worth it for this specific job. For all those other jobs targeted, it takes more than a minute of network requests to download all the dependencies needed (build + scan), and as such the diff is worth it.

PS.2: Dependency cache isn't enabled on the release-builds.yml pipeline just yet, but just temporarily, and until we become comfortable with running this new process on the main pipeline.yml pipeline.


Future Improvements

As per this comment, me and @wzieba has been discussing on DMs about some possible future improvement, which are including but are not limited to the below list:

  1. Only execute the assemble android test task, which depends on assemble task anyway, and would most likely resolve most of the dependencies, for all other tasks to benefit from. However, this needs further testing to make sure that running other tasks such us lint and assemble unit test are providing little to no help.
  2. Create a custom Gradle task and utilize Gradle's init scripts (init.d). Using the init script, go through all available modules for a project, and then, using that custom Gradle task, resolve all dependencies for all tasks that are available per module. We could take some inspiration from plugins such as this.
  3. Instead of scheduling the dependency cache build to run weekly (buildkite-ci#576), make it run more often, possibly scheduling it to even run daily. However, since dependencies are not being updated too often, and, even when they do on that specific week, it would only be for a few of them. This will indeed cause some extra network requests, but most of the dependencies cached would still benefit from the existing and weekly refreshed dependency cache.
  4. Enable dependency cache on the release-builds.yml pipeline.

Testing information

This dependency cache solution was thoroughly tested via this #13170 test-only PR, but to help you asserting that this change is indeed making an impact, plus not causing any trouble for the project, try and follow the below:

Job BT (Without) BT (With) BT (Diff*) NR (Without) NR (With) RC (Without) RC (With) Win
Mobile App 5m 38s 3m 53s - 1m 45s 1m 15s 0s 0s 12s 1m 3s
Wear App 4m 43s 2m 56s - 1m 47s 59s 0s 0s 15s 44s
Lint 6m 9s 4m 6s - 2m 3s 1m 49s 0s 0s 14s 1m 35s
Unit Tests 6m 0s 3m 57s - 2m 3s 1m 31s 0s (ish) 0s 10s 1m 21s
UI Tests 10m 3s 13m 37s + 3m 34s 1m 38s 0s 0s 15s 1m 23s

Note

  • Without: Without Dependency Cache
  • With: With Dependency Cache
  • BT: Build Time
  • NR: Network Requests
  • RC: Restore Cache
  • Win: NR (Without) - RC (With)

Warning

(*): The BT (Diff) presented above is relative, so don't read too much into it, it might not match with the WIN, which is a much more solid number. But still, it is nice to have and compare those two number together. To to be clear about that, although I tried to pick the most representative builds, without and with the dependency cache mechanism in place, there are other factors that can make any build faster, or slower for that matter (like build cache, firebase test lab, etc), so again, don't read too much into it.


Merge Instructions


  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@ParaskP7 ParaskP7 added status: do not merge Dependent on another PR, ready for review but not ready for merge. category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. Core labels Jan 14, 2025
@ParaskP7 ParaskP7 force-pushed the ci/enable-dependency-cache-on-ci-final branch from 6216579 to af97b81 Compare January 14, 2025 12:09
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 14, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit25a6483
Direct Downloadwoocommerce-wear-prototype-build-pr13303-25a6483.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 14, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit25a6483
Direct Downloadwoocommerce-prototype-build-pr13303-25a6483.apk

@ParaskP7 ParaskP7 force-pushed the ci/enable-dependency-cache-on-ci-final branch from af97b81 to 6ba4e7c Compare January 14, 2025 12:29
@ParaskP7 ParaskP7 changed the title [Dependency Cache] Dependency Cache on CI [Dependency Cache] Enable Dependency Cache on CI Jan 14, 2025
@ParaskP7 ParaskP7 force-pushed the ci/enable-dependency-cache-on-ci-final branch from 6ba4e7c to 4475a6a Compare January 14, 2025 15:21
@ParaskP7 ParaskP7 removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jan 15, 2025
FYI: This change is done for testing purposes and until the below
'A8C CI Toolkit' #135 PR gets merged to 'trunk'. When that's
done, the 'a8c-ci-toolkit' will be updated to '3.9.0' instead.

A8C CI Toolkit PR: [Dependency Updates] [Dependency Cache]
Dependency Cache on CI per Project [without GRADLE_RO_DEP_CACHE] #135
- Automattic/a8c-ci-toolkit-buildkite-plugin#135
FYI: This job will be then used by 'buildkite-ci' and configured as a
'buildkite_pipeline_schedule' with a weekly frequency.

PS: The targeted 'pipeline' related jobs are:
- Mobile App
- Wear App
- Lint
- Unit Tests
- Android tests
PS: The targeted 'pipeline' related jobs are:
- Mobile App
- Wear App
- Lint
- Unit Tests
- Android tests

PS: The 'detekt' job isn't targeted because it only takes about 20+
seconds of network request to download all the dependencies needed for
this specific job, which is about the same time it take for the
dependency cache to actually be restored (10+ seconds). Thus, this
optimization is not really worth it for this specific job. For all those
other jobs targeted, it take more than a minute of network requests to
download all the dependencies needed, and as such worth the diff is
worth it.
Release Notes: https://github.com/Automattic/
a8c-ci-toolkit-buildkite-plugin/releases/tag/3.9.0

Related: 4475a6a
@ParaskP7 ParaskP7 force-pushed the ci/enable-dependency-cache-on-ci-final branch from 9820220 to 3547e66 Compare January 15, 2025 09:07
FYI: This change is done for testing purposes and until the below
'A8C CI Toolkit' #138 PR gets merged to 'trunk'. When that's
done, the 'a8c-ci-toolkit' will be updated to '3.9.1' instead.

A8C CI Toolkit PR: [Dependency Cache] Fix Dependency Cache #138
- Automattic/a8c-ci-toolkit-buildkite-plugin#138
Release Notes: https://github.com/Automattic/
a8c-ci-toolkit-buildkite-plugin/releases/tag/3.9.1

Related: 49d771a
@ParaskP7 ParaskP7 added this to the 21.5 milestone Jan 15, 2025
@ParaskP7 ParaskP7 requested a review from wzieba January 15, 2025 13:13
@ParaskP7 ParaskP7 marked this pull request as ready for review January 15, 2025 13:13
.buildkite/commands/save-cache.sh Show resolved Hide resolved
.buildkite/commands/save-cache.sh Outdated Show resolved Hide resolved
.buildkite/pipeline.yml Outdated Show resolved Hide resolved
ParaskP7 and others added 5 commits January 15, 2025 16:59
Co-authored-by: Wojciech Zięba <wojtek.zieba@automattic.com>
When using 'single-cmd' and not using this double quotes structure it is
is producing 'did not find expected key' type of errors.
@ParaskP7 ParaskP7 merged commit cda6ca5 into trunk Jan 17, 2025
15 checks passed
@ParaskP7 ParaskP7 deleted the ci/enable-dependency-cache-on-ci-final branch January 17, 2025 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants