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

[Buildkite] Test Android Staging on ami-0953ca1d949c23a58 -> ami-0f976e0b4de96aa28 #13387

Closed
wants to merge 4 commits into from

Conversation

ParaskP7
Copy link
Contributor

@ParaskP7 ParaskP7 commented Jan 24, 2025

Related: a8c-ci-toolkit-buildkite-plugin##142

AMI Name: android-build-image-6.12.0v1.5-rc-1

Description

This PR changes the Buildkite agent from android to android-staging. This change is not meant to be merged, but rather used to verify that the new AMI ami-0953ca1d949c23a58 works as expected.


Before the Change



(*) Although, note that these numbers might not be very representative because trunk is using a dependency cache that is already outdated (5 days has passed since it was last updated, every Sunday).


After the Change




Conclusions

As expected, using ./gradlew downloadDependencies androidDependencies is:

  1. Simpler, much less configuration and reasoning about which tasks to use.
  2. Scalable, no need to add any more tasks to the save-cache.sh script as the project evolves.
  3. Producing a bigger cache,
    • Before: 628.98 MB
    • After: 950.32 MB
    • Result: + 321.34 MB more data downloaded, which means even less network requests on CI.
  4. Slower, both in saving and restoring the cache, which is the only drawable:

AMI Name: android-build-image-6.12.0v1.5-rc-1
FYI: This change is done for testing purposes only, and in order to
verify that the newly added 'init.gradle.kts' script is working as
expected.

A8C CI Toolkit PR: [Dependency Cache] Test Custom Init Gradle Task
to Download Dependencies #142
- Automattic/a8c-ci-toolkit-buildkite-plugin#142
@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. labels Jan 24, 2025
@dangermattic
Copy link
Collaborator

1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 24, 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
Commit2f4941b
Direct Downloadwoocommerce-wear-prototype-build-pr13387-2f4941b.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 24, 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
Commit2f4941b
Direct Downloadwoocommerce-prototype-build-pr13387-2f4941b.apk

This custom 'downloadDependencies' task is part of the Android AMI,
that is utilized by the 'android' agents, and it is becoming available
to all project via the 'init.gradle.kts' script that is included in it.
@ParaskP7 ParaskP7 requested a review from wzieba January 24, 2025 14:09
@ParaskP7
Copy link
Contributor Author

👋 @wzieba , I just added you as a review on this draft PR, to be closed when we get ready with the new Android AMI, but please, take a look at the description (results). 🙏

TL;DR: Everything is working as expected, a total win, I am happy with the result, with the only drawback that it now takes more time to save and restore the cache.

  • During saving, this is actually irrelevant as the total build time dropped anyway.
  • But, during restoring, you add an additional 5s to 10s, which is indeed something we expected, but worth noticing/thinking about anyway. 👀

Copy link
Contributor

@wzieba wzieba left a comment

Choose a reason for hiding this comment

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

But, during restoring, you add an additional 5s to 10s, which is indeed something we expected, but worth noticing/thinking about anyway. 👀

Yeah, thanks for highlighting this! It's unfortunate and might even change in which jobs we use the dependency cache, but so low cost of maintaining the "save-cache" script across projects is I think worth these 5-10s.

.buildkite/commands/save-cache.sh Outdated Show resolved Hide resolved
@ParaskP7
Copy link
Contributor Author

Yeah, thanks for highlighting this! It's unfortunate and might even change in which jobs we use the dependency cache, but so low cost of maintaining the "save-cache" script across projects is I think worth these 5-10s.

Btw @wzieba , forgot to say that I agree with what you say here, and worse case we could revert back to using the assemble tasks for saving the cache, or just the assembleDebugAndroidTest, and switching between solutions as we see fit. 👍

A new Android AMI got created that now finalizes 'downloadDependencies'
with the 'androidDependencies' task.
@ParaskP7 ParaskP7 changed the title [Buildkite] Test Android Staging on ami-0953ca1d949c23a58 [Buildkite] Test Android Staging on ami-0953ca1d949c23a58 -> ami-0f976e0b4de96aa28 Jan 28, 2025
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. status: do not merge Dependent on another PR, ready for review but not ready for merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants