-
-
Notifications
You must be signed in to change notification settings - Fork 4
E2e env actions #85
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
base: main
Are you sure you want to change the base?
E2e env actions #85
Conversation
For self hosted nodes we can decoupling some steps.
| aws-role-to-assume: ${{ inputs.keystore-role-to-assume }} | ||
| aws-region: 'us-east-2' | ||
| platform: 'ios' | ||
| environment: ${{ inputs.environment }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Keystore Configuration Ignores Required Secret Name
The configure-keystore action's secret-name input is marked as required but is not provided by its caller (setup-e2e-env), leading to a workflow failure. Additionally, the configure-keystore action ignores this input, dynamically determining the secret name from the environment input instead.
Additional Locations (1)
| ## IOS Setup ## | ||
| - name: Configure iOS Signing Certificates | ||
| if: ${{ inputs.platform == 'ios' && inputs.configure-keystores == 'true' }} | ||
| uses: MetaMask/github-tools/.github/actions/configure-keystore@e2e-env-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Local Action Reference Error
The setup-e2e-env action incorrectly references an external MetaMask/github-tools/.github/actions/configure-keystore@e2e-env-actions (lines 144, 233). This change introduces a local configure-keystore action, which should be used instead, as the external reference is likely invalid.
Additional Locations (1)
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Toolchain Path Not Platform-Agnostic
The NDK toolchain path in the 'Add NDK related toolchains to PATH' step is hardcoded to linux-x86_64. This prevents the action from correctly setting up the NDK on non-Linux runners (e.g., macOS, Windows), as the prebuilt directory varies by host platform (e.g., darwin-x86_64 for macOS, windows-x86_64 for Windows). The path should dynamically adapt to the runner.os.
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" | ||
| echo "$NDK_TOOLCHAIN" >> "$GITHUB_PATH" | ||
| echo "$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Path and SDK Environment Mismatch
The NDK toolchain path hardcodes linux-x86_64, which will cause Android builds to fail on macOS runners where darwin-x86_64 is required. The path should dynamically determine the correct operating system and architecture. Additionally, there is inconsistent use of ANDROID_SDK_ROOT and ANDROID_HOME throughout the Android setup, which can lead to path resolution issues.
| required: true | ||
| secret-name: | ||
| description: 'The name of the secret in AWS Secrets Manager' | ||
| required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Unused Required Input Causes Misleading Configuration
The secret-name input is defined as required but is never used by the action. Instead, the secret name is dynamically determined from the environment input via a hardcoded mapping. This makes the secret-name input misleading and forces callers to provide an unnecessary value.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SECRET_NAME="metamask-mobile-main-signing-certificates" | ||
| ;; | ||
| *) | ||
| echo "❌ Unknown environment: ${{ inputs.environment }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## IOS Setup ## | ||
| - name: Configure iOS Signing Certificates | ||
| if: ${{ inputs.platform == 'ios' && inputs.configure-keystores == 'true' }} | ||
| uses: MetaMask/github-tools/.github/actions/configure-keystore@e2e-env-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Action References External Path Incorrectly
The setup-e2e-env action incorrectly references the configure-keystore action using an external path (MetaMask/github-tools/.github/actions/configure-keystore@e2e-env-actions). Since configure-keystore is defined locally within this repository, it should be referenced via a relative path (e.g., ../configure-keystore). This issue occurs in multiple instances.
Additional Locations (1)
| ## Launch AVD | ||
|
|
||
| - name: Set ANDROID_AVD_HOME for downstream steps | ||
| if: ${{ inputs.platform == 'android'}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Toolchain Path Issue on macOS
The NDK toolchain path is hardcoded to linux-x86_64, preventing the action from working on macOS runners where darwin-x86_64 is required. Additionally, there is inconsistent spacing in if conditions, specifically missing a space before the closing braces }} for Android-related steps.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SECRET_NAME="metamask-mobile-main-signing-certificates" | ||
| ;; | ||
| *) | ||
| echo "❌ Unknown environment: ${{ inputs.environment }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect Error Message Variable Reference
The Determine signing secret name step's error message incorrectly references ${{ inputs.environment }} instead of ${{ inputs.target }}. This causes the error message to display an empty value when an unknown target is provided, rather than the actual invalid input.
| SECRET_NAME="metamask-mobile-main-signing-certificates" | ||
| ;; | ||
| *) | ||
| echo "❌ Unknown environment: ${{ inputs.environment }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - name: Select Xcode version | ||
| if: ${{ inputs.platform == 'ios' }} | ||
| run: sudo xcode-select -s /Applications/Xcode_${{ inputs.xcode-version }}.app/Contents/Developer | ||
| shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Redundant Xcode Version Selection in CI
The setup-e2e-env action contains duplicate Xcode version selection steps for iOS. The first step (line 144) uses an incomplete path and lacks the shell: bash directive, while the second step (line 196) uses the correct path and includes the shell directive. The second step overrides the first, rendering the initial selection redundant.
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" | ||
| echo "$NDK_TOOLCHAIN" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Toolchain Path Incorrect on Non-Linux Runners
The NDK toolchain path is hardcoded to linux-x86_64 in the Android setup step. This causes an incorrect path to be added to GITHUB_PATH on non-Linux x86_64 runners (e.g., macOS, where darwin-x86_64 is expected, or ARM64 runners), leading to NDK toolchain setup failures.
| "platforms;android-${{ inputs.android-api-level }}" \ | ||
| "build-tools;34.0.0" \ | ||
| "emulator" \ | ||
| "system-images;android-${{ inputs.android-api-level }};google_apis;${{ inputs.android-abi }}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| SECRET_NAME="metamask-mobile-main-signing-certificates" | ||
| ;; | ||
| *) | ||
| echo "❌ Unknown environment: ${{ inputs.environment }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # path: ios/Pods | ||
| # key: ${{ inputs.cache-prefix }}-pods-${{ inputs.platform }}-${{ runner.os }}-${{ hashFiles('ios/Podfile.lock') }} | ||
| # restore-keys: | | ||
| # ${{ inputs.cache-prefix }}-pods-${{ inputs.platform }}-${{ runner.os }}- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Redundant Xcode Selection Steps
The setup-e2e-env action contains two "Select Xcode version" steps for iOS. The first step (lines 142-145) uses an incorrect path (/Applications/Xcode_X.X.app), which is immediately overridden by the second step (lines 195-198) that uses the correct path (/Applications/Xcode_X.X.app/Contents/Developer), making the first step redundant. Additionally, there is commented-out code for Xcode selection and CocoaPods cache restoration, indicating leftover development/debugging code.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" | ||
| echo "$NDK_TOOLCHAIN" >> "$GITHUB_PATH" | ||
| echo "$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}" >> "$GITHUB_PATH" | ||
| shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - name: Select Xcode version | ||
| if: ${{ inputs.platform == 'ios' }} | ||
| run: sudo xcode-select -s /Applications/Xcode_${{ inputs.xcode-version }}.app/Contents/Developer | ||
| shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: iOS Xcode Selection Fails on GitHub Runners
The setup-e2e-env action includes duplicate Xcode version selection steps for iOS. A step conditional for self-hosted runners and another unconditional step for all iOS platforms lead to self-hosted runners executing xcode-select twice. Additionally, the unconditional step attempts to select Xcode using a path that may not exist on GitHub-hosted runners, potentially causing build failures. This indicates an incorrect implementation of intended distinct Xcode paths for self-hosted versus GitHub-hosted environments.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Toolchain Path Incorrect on macOS
The NDK toolchain path is hardcoded to linux-x86_64 in the Add NDK related toolchains to PATH step. This causes Android setup to fail on macOS runners, as the correct path for macOS is darwin-x86_64. The path should be determined dynamically based on runner.os.
| # path: ios/Pods | ||
| # key: ${{ inputs.cache-prefix }}-pods-${{ inputs.platform }}-${{ runner.os }}-${{ hashFiles('ios/Podfile.lock') }} | ||
| # restore-keys: | | ||
| # ${{ inputs.cache-prefix }}-pods-${{ inputs.platform }}-${{ runner.os }}- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" | ||
| echo "$NDK_TOOLCHAIN" >> "$GITHUB_PATH" | ||
| echo "$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}" >> "$GITHUB_PATH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NDK Path Issues Across Platforms
The Android NDK toolchain path is hardcoded to linux-x86_64, which is incorrect for macOS runners where darwin-x86_64 is required. This path also inconsistently uses $ANDROID_SDK_ROOT instead of $ANDROID_HOME, which is used by other Android setup steps, potentially leading to incorrect NDK paths.
| - name: Add NDK related toolchains to PATH | ||
| if: ${{ inputs.platform == 'android' }} | ||
| run: | | ||
| NDK_TOOLCHAIN="$ANDROID_SDK_ROOT/ndk/${{ inputs.ndk-version }}/toolchains/llvm/prebuilt/linux-x86_64/bin" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Introduce Reusable E2E Environment Setup GitHub Action
📂 What’s new
This PR introduces a reusable composite GitHub Action at
.github/actions/setup-e2e-envdesigned to standardize and streamline the setup of E2E test environments across iOS and Android platforms.✅ Features
🧰 Common Tooling Setup
node-versioninput)yarn-versionvia Corepack)foundryup)node_modules(Yarn)vendor/bundle)Pods/)🍎 iOS Platform Support (
platform: ios)ruby-version)bundler-version) and gem resolution viabundle installxcode-version)bundle exec pod installsetup-simulator)ios-simulator-device🤖 Android Platform Support (
platform: android)jdk-versionandjdk-distribution)platform-tools,build-tools, emulator, system imagesndk-version)aptplatform-tools,emulator,cmdline-tools)🧪 Testing
Verified on:
macos-14andmacos-14-largerunnersExample usage:
📥 Inputs
platformiosorandroid(required)node-version20.18.0)yarn-version1.22.22)setup-simulatorfalse)ios-device"iPhone 15")bundler-version2.5.8)cache-prefixe2e)ruby-version3.1)xcode-version16.2)jdk-version17)jdk-distributiontemurin)ndk-version26.1.10909125)foundry-versionv1.2.3)android-deviceandroid-api-level34)android-abix86_64)📌 Notes
platform: androidand vice versa.PATHfor CLI-based native tools likeclangorndk-build.NODE_OPTIONS=--max-old-space-size=4096.