Conversation
Add a GitHub Actions workflow (upgrade_tests.yml) to run upgrade verification on a self-hosted macOS runner: it creates a simulator, installs/builds an old tagged release to create a wallet, then checks out the current SHA and verifies the wallet survives the upgrade. Add a new UITest (UpgradeVerificationTests.swift) that asserts onboarding is skipped and the Bitcoin receive flow still shows the expected address. Update Gem.xcodeproj to include the new test file and extend the justfile to add test-upgrade-setup and test-upgrade-verify targets and allow running _test-ui for a specific test target.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes a robust automated upgrade verification process for the iOS application. By integrating a new GitHub Actions workflow, a dedicated UI test, and Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces an upgrade test workflow and a new UI test to verify wallet data persistence across upgrades. The changes to the justfile to support running these tests are a good addition.
I have a couple of suggestions:
- In
UpgradeVerificationTests.swift, I recommend replacing hardcoded strings for UI identifiers with constants to improve maintainability. - In the
justfile, thetest-upgrade-setuprecipe should probably ensure the simulator is reset to provide a clean state for the first phase of the upgrade test.
Overall, this is a valuable addition for ensuring application stability during upgrades.
Add a pull_request trigger scoped to the main branch to exercise the upgrade_tests workflow on this branch. A TODO notes that the pull_request trigger should be removed before merging; workflow_dispatch inputs are unchanged.
Update the upgrade_tests GitHub Actions workflow to use explicit xcodebuild commands for resolving packages, building UI tests, and running a targeted test, replacing several just tasks. Bump the default old_version input (and related env/job name) from 1.3.327 to 1.3.335 and clarify the input description (must be >= 1.3.328). Reorganize and rename steps to separate Phase 1 (build/run old version to create wallet) and Phase 2 (build/run current version to verify), and pipe xcodebuild output through xcbeautify for cleaner CI logs.
Improve stability of UpgradeVerificationTests by waiting for UI elements before interacting with them. Store buttons in local variables and assert their existence with timeouts (10s for asset and action buttons, 5s for the address) to reduce flakiness when tapping elements after an app upgrade.
Refines tests to improve clarity and stability. In UpgradeVerificationTests the Bitcoin asset assertion message was updated to mention 'after upgrade' and a subsequent bitcoin address existence assertion was removed (to avoid a brittle check). In WalletServiceTests the WalletService.mock is now created with an injected walletStore backed by a mocked DB pre-populated with the Ethereum chain so the test has the expected initial state for deleteLastWalletNotifiesObservers.
Replace the old Receive->SelectAsset->Copy flow with a Wallet-detail based verification. The test now navigates to the Wallets screen, opens wallet settings (gearshape), taps Show Secret Phrase, proceeds through the security reminder, and verifies the displayed secret phrase words against UITestKitConstants. This adapts the test to the updated UI and ensures the wallet keys were not corrupted after upgrade.
…integration-test-for-ios-app-upgrade-flow-from-1-version-to-another # Conflicts: # Gem.xcodeproj/project.pbxproj
Remove GitHub Actions workflow and consolidate upgrade test into a single `just test-upgrade <commit>` command that uses git worktree for isolation.
0c5f468 to
ec134c6
Compare
…integration-test-for-ios-app-upgrade-flow-from-1-version-to-another
build-system/scripts/upgrade-test.sh
Outdated
| -allowProvisioningUpdates \ | ||
| -allowProvisioningDeviceRegistration \ | ||
| -parallelizeTargets \ | ||
| -jobs "$BUILD_THREADS" \ |
There was a problem hiding this comment.
| -jobs "$BUILD_THREADS" \ |
why you need this? it should be default
build-system/scripts/upgrade-test.sh
Outdated
| ONLY_ACTIVE_ARCH=YES \ | ||
| -destination "$SIMULATOR_DEST" \ | ||
| -derivedDataPath "$DERIVED_DATA" \ | ||
| -clonedSourcePackagesDirPath "$SPM_CACHE" \ |
There was a problem hiding this comment.
| -clonedSourcePackagesDirPath "$SPM_CACHE" \ |
don't use cache for this testing
Update build-system/scripts/upgrade-test.sh to remove explicit SPM cache and parallel build settings. Deleted OLD_SPM_CACHE and SPM_CACHE variables and removed BUILD_THREADS. Also removed -clonedSourcePackagesDirPath, -parallelizeTargets and -jobs from xcodebuild invocations, simplifying the script's build/test commands.
…integration-test-for-ios-app-upgrade-flow-from-1-version-to-another
Local upgrade test via
just test-upgrade <commit>:Usage
Close: #1629