Skip to content

Conversation

@AliSoftware
Copy link
Contributor

@AliSoftware AliSoftware commented Feb 6, 2023

What?

Improve Prototype (aka Installable) Builds in a couple of ways:

  • Make the PR comment announcing the Prototype build QR code and information nicer
    • In particular, the QR code and URLs now directly links to the appropriate build on App Center, instead of defaulting to show the latest build in App Center as before
  • Change the wording used throughout Fastfile/Buildkite from "Installable Build" to "Prototype Build".
    • Every build is "installable" in some way, so that wording didn't make much sense. We're already using the term "Prototype Builds" for those in other apps, which feels like a better fit already, so we've decided to slowly adopt this new wording for those from now on to us make the term less ambiguous and more descriptive.
  • Attach Buildkite metadata to the CI build with relevant infos
  • Add a Buildkite annotation to the CI build with relevant infos
  • Add more info to the App Center "Release Notes" field of each Prototype Builds being created

To test:

  • Check that the PR got two PR comments, providing the information for the WordPress and Jetpack Prototype builds respectively, unfold the triangle/spoiler blocks, and verify that the information displayed for those builds is accurate
  • Scan the QR code (and/or follow the URL) and validate that it directly redirects you to the page in App Center that corresponds to the right build, even if new builds have been uploaded to App Center since and this is not the latest one anymore
  • Verify that the "Release Notes" field in App Center for that build contains the appropriate information about the Commit and PR it corresponds to
  • Open the Buildkite build for this PR, and check that the annotation shows some metadata about the App Center build (short/long version, app center id, …)
  • Add /meta-data to the URL of the Buildkite build and validate that it includes the correct metadata
  • Use the ?meta_data query parameter on the Buildkite URL, e.g. ?meta_data[build_type]=Prototype to search for all CI builds that correspond to / include Prototype builds

@AliSoftware AliSoftware self-assigned this Feb 6, 2023
@AliSoftware AliSoftware added the Tooling Build, Release, and Validation Tools label Feb 6, 2023
@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch from 520a13e to 342d3b7 Compare February 6, 2023 19:41
@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch 3 times, most recently from f2fc2bf to e951c84 Compare February 7, 2023 12:50
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 7, 2023

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr20071-59a3f8e
Version21.9
Bundle IDorg.wordpress.alpha
Commit59a3f8e
App Center BuildWPiOS - One-Offs #5146
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 7, 2023

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr20071-59a3f8e
Version21.9
Bundle IDcom.jetpack.alpha
Commit59a3f8e
App Center Buildjetpack-installable-builds #4168
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch 2 times, most recently from 2963cd0 to 6875032 Compare February 7, 2023 22:43
@mokagio
Copy link
Contributor

mokagio commented Feb 7, 2023

I was about to comment in regards to the output_app_name CI failure, but I saw your force push should address it 👍

@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch from 6875032 to db495d6 Compare February 7, 2023 23:00
@AliSoftware
Copy link
Contributor Author

AliSoftware commented Feb 8, 2023

👌
image

Note: I cancelled the "Build for Testing" step on CI to avoid spawning Mac workers for it needlessly (as this PR is still Draft and will have more force-pushes to come—especially to update the reference in the Gemfile once the new toolkit is released—and our Mac queue is already under enough stress as it is recently)

 - DRY the common steps for WordPress vs Jetpack into a helper method
 - Use latest release-toolkit action to generate a nice-looking PR comment with the QR code and build info
@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch 3 times, most recently from 45ae889 to 5b4cb22 Compare March 8, 2023 15:54
@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch 2 times, most recently from 3b38f53 to 355fe18 Compare March 8, 2023 18:22
Every build can be "installed", so the term "Installable Build" wasn't super descriptive.
We are already calling them "Prototype Builds" in other repos, so we decided to adopt that new wording everywhere from now on.
@AliSoftware AliSoftware force-pushed the tooling/prototype-build-metadata branch from 355fe18 to 59a3f8e Compare March 8, 2023 19:16
@AliSoftware AliSoftware marked this pull request as ready for review March 8, 2023 20:00
@AliSoftware AliSoftware requested review from a team and mokagio March 8, 2023 20:02

echo "--- :hammer_and_wrench: Building"
bundle exec fastlane build_and_upload_jetpack_installable_build
bundle exec fastlane build_and_upload_wordpress_prototype_build
Copy link
Contributor

Choose a reason for hiding this comment

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

I love how git got confused about the renaming and ended up producing this diff.

buildkite_metadata(set: metadata)
appcenter_install_url = "https://install.appcenter.ms/orgs/#{APPCENTER_OWNER_NAME}/apps/#{appcenter_app_name}/releases/#{appcenter_id}"
list = metadata.map { |k, v| " - **#{k}**: #{v}" }.join("\n")
buildkite_annotate(context: "appcenter-info-#{output_app_name}", style: 'info', message: "#{output_app_name} [App Center Build](#{appcenter_install_url}) Info:\n\n#{list}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Occasionally I noticed there are "Test Failures" annotation in the build page, but all test jobs are successful. I think the root cause is the "Test Failures" annotation isn't removed after a successful retry.

I think a similar issue may occur here too: this "build info" annotation won't be removed if the latest build fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That fastlane action is called at the very end of the script and buildkite step doing the Prototype build. So an annotation will only be added if the Prototype build step ran all the way to the end once. But if it went all the way to the end without an issue, the CI step will be green and Buildkite won't even show a button to retry the step, so it's impossible for that step to succeed first (and add metadata) then fail later.

(and if the user chooses "Rebuild" on the whole CI build, that creates an entirely separate build and Buildkite page for it anyway)

The only case where I can see the situation that you describes happening is if the script and lane called by that CI step goes all the way to the end, after successfully pushing the build to App Center and adding the annotation, but fail at some meta CI level (eg failure to terminate and unregister the VM properly or whatnot)… and then you retry the step, and then this time if fails before getting to upload a new App Center build and adding the annotation… quite the corner case imho 😅


In contrast, the reason why annotations for tests stay around even after build success is a bit different:

  • The test failure annotations are generated on step failure, in case where we can (and are likely to) retry the step. While App Center metadata info annotations are added on success
  • The annotation for test failure annotation is handled by a shell script from our a8c-ci-toolkit-buildkite-plugin (annotate_test_failures action iirc?) not by the fastlane action from our release-toolkit
  • One of the reasons for why we sometimes still have those annotations reporting test failures even when the step ended up green in the end… might not be because we forgot to remove the annotation, but because the test was flaky but Xcode was set up to do 3 retries before marking it as failed, but still logged every attempt in the report.junit (with one XML node for the initial failure and another for the ultimate success for the same test) but our annotate_test_failures a8c-ci-toolkit command is not smart enough to pick up on that (we'll have to debug and implement a fix for all that at some point)

@AliSoftware AliSoftware merged commit c9e9fe8 into trunk Mar 9, 2023
@AliSoftware AliSoftware deleted the tooling/prototype-build-metadata branch March 9, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Tooling Build, Release, and Validation Tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants