Repository navigation
ci: integration tests finish in about 15 minutes - #1046
Conversation
… failed tests alone
[Low impact] The changes affect one CI integration-test lane and its UTXO tests. The PR appears safe to merge, with a non-blocking cleanup needed to stop tests after a polling timeout. Findings
SummaryThe PR separates the integration build from test execution and replaces full-suite retries with one retry per failed test.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
Restore[Restore package cache] --> Resolve[Resolve locked packages]
Resolve --> Save[Save package cache]
Save --> Build[Build for testing once]
Build --> Test[Run integration tests]
Test --> Retry[Retry failed tests once]
Retry --> Upload[Upload test results]
Reviews (1) · Last reviewed commit: "ci: integration tests build once, cache ..." · Reviewed by Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fedada213
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
talosmachina
left a comment
There was a problem hiding this comment.
1 finding (1 P1). Caches SwiftPM checkouts and artifacts only, builds once with build-for-testing, retries a failed test on its own, and replaces fixed sleeps in UtxoSelectionTests with polling. Reviewed 5fedada, light tier.
What I checked, and 2 candidates I ruled out
Read in full: .github/workflows/integration-tests.yml at the head, the UtxoSelectionTests.swift change
Checked against: SwiftPM release/6.1 and release/6.2 sources (Workspace+Dependencies.swift, RepositoryManager.swift, GitRepository.swift), plus a Swift 6.1.3 reproduction for the finding
CI: this run is a cache miss: Resolve took 7.7 min and Build for testing was still running. SwiftFormat is red on ElectrumSettingsScreen.swift, RgsSettingsScreen.swift and ActivityRow.swift, which this PR does not touch; that lint workflow is red on master too.
Not built here: iOS does not build on this box, so this is reasoned from the code, from SwiftPM's source and from CI. A cold reader from another model family had not finished with the finding when this was posted.
Ruled out
- Retry masks a real failure:
-retry-tests-on-failure -test-iterations 2reruns only the failed test, and xcodebuild still exits non-zero when both iterations fail. - Save step caches a half-resolved tree: it runs only on a cache miss and only after the resolve step succeeded, since
set -o pipefailfails that step first.
Merge confidence: 3/5, the P1 means the main speedup may not appear on a warm cache, and the test lane had not finished.
|
Pushed ee94d1c: deletes The SwiftFormat failure comes from master's |
|
Pushed 1772609: caches the package clones too, cut down to the pinned commits by |
|
Pushed 68beb61 and merged master (78ad25c), so SwiftFormat reruns on master with #1040 in. 68beb61 boots the simulator at the start of the job and waits for it before testing. On the 1772609 run all tests were passing when the 20-minute step limit hit, but the test step had spent 13 of those minutes before the first test: 10 waiting for the fresh iPhone 16 simulator, then 3 bootstrapping it. Booting it first overlaps that with resolving and building. The destination is now that simulator's id (the same latest-runtime iPhone 16 that Timings on 1772609, cold cache: resolve 10.5 min, shrinking the clones 14 s, saving the cache 15 s, build 5.8 min. |
…tays under the cache limit
On 78ad25c the early boot worked: the simulator was ready when the test step began, and bootstrapping took 21 s instead of 3 min. Its tests then hit a Blocktank staging outage: every regtest mine and deposit call returned HTTP 500 or failed to connect, and |
jvsena42
left a comment
There was a problem hiding this comment.
Approving. No issues found. Integration run 39468042023 is green at 39555da.
Checked and clean:
- The timeouts at
1772609and78ad25c:test-without-buildingprinted nothing for 7.6-9.8 min before the destination line, because it lacked-clonedSourcePackagesDirPathand re-cloned every package into DerivedData.39555daadds the package flags (integration-tests.yml:97-100), and the head run reaches the destination line 15 s after invocation.68beb61moves the simulator boot out of the capped step. - The 20-minute cap: across 11 master runs, test execution takes 9.9-12.5 min with a booted simulator, and one retry of the flaky suites adds about 2.5 min. That puts the worst measured case near 16 min. The head run's test step took 12.0 min.
timeout-minutes: 25would leave room for two retries if you want the margin. - Composite action:
SOURCE_PACKAGESis exported before use.- The key includes the Xcode version, and there are no restore-keys.
- The cache is saved only on a miss after a successful resolve, and a concurrent save only warns.
- Every later xcodebuild call in the unit, e2e, staging and migration workflows passes the package flags.
- Dropping "Clean build" is safe on a fresh runner, and the test assertions are unchanged.
@talosmachina's re-clone question isn't settled yet. The head run missed the cache (13.75 min cold resolve), and its save collided with the unit-tests job, which did save the key. A re-run of the integration job now should hit the cache. If its "Resolve Swift packages" takes under a minute, that settles it. Master's 17 GB macOS-spm-* entries can evict the 658 MiB entry again until this merges.
Device gate: no device check needed (CI only); perf: CI wall time only, cold-cache job 40 min (packages 15, build 10.2, tests 12); l10n: no UI text changed.
|
@jvsena42 The re-run settles most of it. Attempt 2 of run 39468042023 at What I have not seen: how those 2 min 49 s split between the restore and "Build for testing" took 9 min 16 s on the same run, and the test step started at 18:37:54Z. I did not wait for it to finish. |
The warm-cache rerun of 39555da passed: the package step took 2.8 minutes, with resolving down to 44 s from 10.5–13.75 minutes cold, and the job took 29 minutes against 52–54 before. The description has the full table. |
jvsena42
left a comment
There was a problem hiding this comment.
Approving again. Both new commits are clean.
59385f9:timeout-minutes: 25on the integration test step. That leaves room for two retries over the 9.9-12.5 min measured test time.eedf7c4:waitForSpendableOutputsnow throwsPollTimeoutinstead of recordingXCTFailand returning. A test whose funding never confirmed stops before it spends. No assertion is weakened, and this answers greptile'sUtxoSelectionTests.swift:42thread.
My earlier review's trace stands. @talosmachina's re-clone question still needs one cache-hit run of the integration job.
Device gate: no device check needed (CI and tests only); perf: CI wall time only; l10n: no UI text changed.
|
@talosmachina the split, from attempt 2's step log:
The test step then took 15.5 min (18:37:54 to 18:53:22): 34 tests, no failures, no retries, 11.8 min in the suite. The job took 29 min; the description's Timings table has it next to the cold run and the runs from before this PR. |
|
@jvsena42 The re-clone question is closed. The cache-hit run is attempt 2 of run 39468042023 at |
|
Merged master (36bd5b1) to sit on top of #923. It merged cleanly: #923 moved the e2e test job to Namespace runners, and the package cache change is in the On 59385f9 every required check except |
Description
-clonedSourcePackagesDirPath, with each package's bare clone cut down to its pinned commit byscripts/shallow-source-packages.sh(18 GB down to about 2 GB, most of it the binary xcframeworks). The old entry cached~/Library/Caches/org.swift.swiftpm(17 GB, mostly bitkit-core's full git history), over GitHub's 10 GB repository cache limit, so it never restored and resolving took 8–12 minutes on every run. SwiftPM 6.1 (Xcode 16.4) looks up every package's clone on each resolve, so the clones are cached too, not only the checkouts..github/actions/swift-packages, keyed by Xcode version (16.4 and 26.2 lanes each get one entry of about 2 GB), and uses it in every workflow that cached SwiftPM. The repository cache stood at 20 GB against GitHub's 10 GB limit, so as long as any workflow still saved a 17 GBmacOS-spm-*entry, the integration lane's new entry was evicted before the next run:integration-tests.yml: uses the action.unit-tests.yml: uses the action, passes the package flags toxcodebuild test, and drops itsClean buildstep, which would resolve again into DerivedData.e2e-tests.yml,e2e-staging.yml,e2e_migration.yml: the app build job uses the action and passes the package flags toxcodebuild build. On top of ci: move Mac mini jobs to Namespace runners #923, which moved the e2e test job to Namespace runners and left thebuild-localjob these steps sit in unchanged.build-for-testingand runstest-without-buildingwith-retry-tests-on-failure -test-iterations 2, so a flaky live regtest test is retried on its own. The old step erased the simulator and rebuilt and reran the whole suite up to 3 times, about 15 minutes per attempt, until the 60-minute job timeout.Clean buildstep, which does nothing on a fresh runner.UtxoSelectionTestspolls sync and spendable outputs until the deposits confirm or the spent UTXOs disappear, instead of sleeping a fixed 15 s or 10 s and syncing once, which saw partial state on CI (4 of 5 UTXOs) and failed on a singleWalletOperationTimeout. A poll that times out throws, so the test stops before it spends.Out of Scope
PaymentFlowTests,BlocktankRefundAddressLiveIntegrationTestsandAddressTypeIntegrationTestsbeyond the per-test retry.Design
N/A — no UI changes.
Preview
N/A — no user-visible changes.
QA Notes
Journeys
N/A — no user-visible behaviour change.
Manual Tests
N/A
Automated Checks
shallow-source-packages.sh— resolve and build make no network fetch from the shrunk clones (checked offline with Swift 6.1.3)UtxoSelectionTests.swift— waits for confirmed deposits and spent UTXOs by polling instead of fixed sleepsintegration-tests.yml— timings belowTimings
Run Integration Tests job:
The package cache is 658 MB per Xcode version, down from 17 GB. Until other branches pick up this change, their runs still save 17–18 GB
macOS-spm-*entries that can evict it, as happened on 59385f9's run. Most of the remaining time is the build, which is not cached, and the live regtest suites (about 12 minutes on a booted simulator).