Skip to content

ci: integration tests finish in about 15 minutes - #1046

Merged
ovitrif merged 10 commits into
masterfrom
ci/faster-integration-tests
Oct 20, 2026
Merged

ovitrif merged 10 commits into
masterfrom
ci/faster-integration-tests

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Description

  • Caches SwiftPM's packages in a fixed -clonedSourcePackagesDirPath, with each package's bare clone cut down to its pinned commit by scripts/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.
  • Saves the package cache right after resolving, so a failing test run still leaves it warm for the next one.
  • Moves restore, resolve, shrink and save into one local action, .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 GB macOS-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 to xcodebuild test, and drops its Clean build step, 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 to xcodebuild build. On top of ci: move Mac mini jobs to Namespace runners #923, which moved the e2e test job to Namespace runners and left the build-local job these steps sit in unchanged.
  • Builds once with build-for-testing and runs test-without-building with -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.
  • Drops the Clean build step, which does nothing on a fresh runner.
  • Boots the simulator at the start of the job, so its boot (over 10 minutes for a fresh one on the runner) overlaps with resolving and building instead of stalling the test step.
  • Caps the test step at 25 minutes, room for two retries of a flaky suite.
  • UtxoSelectionTests polls 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 single WalletOperationTimeout. A poll that times out throws, so the test stops before it spends.

Out of Scope

  • Moving the integration lane to Namespace runners.
  • Hardening PaymentFlowTests, BlocktankRefundAddressLiveIntegrationTests and AddressTypeIntegrationTests beyond 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

  • added shallow-source-packages.sh — resolve and build make no network fetch from the shrunk clones (checked offline with Swift 6.1.3)
  • updated UtxoSelectionTests.swift — waits for confirmed deposits and spent UTXOs by polling instead of fixed sleeps
  • updated integration-tests.yml — timings below

Timings

Run Integration Tests job:

Run Packages Build Tests Job
Before: master, failed 7.7 min in tests 45 min, 3 full attempts 54 min
Before: PR, passed 11.7 min in tests 40 min 52 min
After: cold cache, passed 15.1 min 10.2 min 12.0 min 40 min
After: warm cache, passed 2.8 min (resolve 44 s) 9.3 min 15.5 min 29 min

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).

@ovitrif ovitrif added this to the 3.1.0 milestone Oct 20, 2026
@ovitrif ovitrif self-assigned this Oct 20, 2026
@greptile-apps

greptile-apps Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[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

  1. P2 Timed-out tests keep spending ▶

Summary

The PR separates the integration build from test execution and replaces full-suite retries with one retry per failed test.

  • The integration-test workflow caches packages and runs tests in a single build.
  • UTXO tests wait until the wallet shows the expected state.

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]
Loading

Reviews (1) · Last reviewed commit: "ci: integration tests build once, cache ..." · Reviewed by Greptile

Comment thread BitkitTests/UtxoSelectionTests.swift Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread .github/workflows/integration-tests.yml Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T15:40:13.729165Z 5fedada PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 2 reruns 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 pipefail fails 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.

Comment thread .github/workflows/integration-tests.yml Outdated
@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed ee94d1c: deletes SourcePackages/repositories after the cache save. The first run resolved packages cold (7.7 min) and built (5.7 min), then the runner ran out of disk during the tests: the full package clones (bitkit-core's history alone is over 13 GB) are only needed to resolve, not to build.

The SwiftFormat failure comes from master's ElectrumSettingsScreen.swift and RgsSettingsScreen.swift, which #1040 fixes.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 1772609: caches the package clones too, cut down to the pinned commits by scripts/shallow-source-packages.sh, and drops the step that deleted them. On Xcode 16.4 SwiftPM looks up every clone on each resolve, so caching only checkouts and artifacts re-cloned bitkit-core's full history on a cache hit (raised in #1046 (comment)). The cache key changed, so the next run fills the cache and the one after shows the warm timings.

@ovitrif
ovitrif requested review from a team, ben-kaufman and pwltr and removed request for a team October 20, 2026 16:17
@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

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 OS=latest picked).

Timings on 1772609, cold cache: resolve 10.5 min, shrinking the clones 14 s, saving the cache 15 s, build 5.8 min.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed e3b8f0c and 39555da.

  • e3b8f0c moves the package steps into a local action, .github/actions/swift-packages, keyed by Xcode version, and uses it in unit-tests.yml, e2e-tests.yml, e2e-staging.yml and e2e_migration.yml too. The repository cache sits at 20 GB against the 10 GB limit while those workflows save 17 GB macOS-spm-* entries, so 78ad25c's run found this lane's entry already evicted and ran cold again. The description lists the change per workflow.
  • 39555da passes the package flags to test-without-building. Without them it re-resolved into DerivedData, which cost 7.5 minutes before the first test on 78ad25c.

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 testGetChannelFundableBalanceExcludesLegacy spent 10 minutes across its two iterations before the step limit.

@ovitrif ovitrif changed the title ci: integration tests build once, cache only package checkouts, retry failed tests alone ci: integration tests finish in about 15 minutes Oct 20, 2026
jvsena42
jvsena42 previously approved these changes Oct 20, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. No issues found. Integration run 39468042023 is green at 39555da.

Checked and clean:

  • The timeouts at 1772609 and 78ad25c: test-without-building printed nothing for 7.6-9.8 min before the destination line, because it lacked -clonedSourcePackagesDirPath and re-cloned every package into DerivedData. 39555da adds the package flags (integration-tests.yml:97-100), and the head run reaches the destination line 15 s after invocation. 68beb61 moves 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: 25 would leave room for two retries if you want the margin.
  • Composite action:
    • SOURCE_PACKAGES is 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.

@talosmachina

Copy link
Copy Markdown

@jvsena42 The re-run settles most of it. Attempt 2 of run 39468042023 at 39555da restored macOS-xcode-16.4-source-packages-v2-3856… (690 MB, last accessed 18:26:52Z), and the whole "Swift packages" step took 2 min 49 s (18:25:49 to 18:28:38). The cold resolve on the first attempt took 13.75 min. A run that re-cloned bitkit-core's full history could not finish in that time, so the cached shallow clones are being used and my P1 from 5fedada is resolved by 1772609.

What I have not seen: how those 2 min 49 s split between the restore and -resolvePackageDependencies. The job was still running when I checked, so its log was not downloadable yet. If the split matters, the step log of this attempt will show it once the job ends.

"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.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed eedf7c4 and 59385f9.

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.

@ovitrif
ovitrif requested a review from jvsena42 October 20, 2026 18:55

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving again. Both new commits are clean.

  • 59385f9: timeout-minutes: 25 on the integration test step. That leaves room for two retries over the 9.9-12.5 min measured test time.
  • eedf7c4: waitForSpendableOutputs now throws PollTimeout instead of recording XCTFail and returning. A test whose funding never confirmed stops before it spends. No assertion is weakened, and this answers greptile's UtxoSelectionTests.swift:42 thread.

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.

@ovitrif
ovitrif enabled auto-merge October 20, 2026 19:00
@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@talosmachina the split, from attempt 2's step log:

  • Restore: 2 min 4 s (18:25:49 to 18:27:53). The 658 MB download itself ran at about 15 MB/s; the rest is the action fetching the entry.
  • -resolvePackageDependencies: 44 s (18:27:53 to 18:28:37), with no fetch or clone lines.

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.

@talosmachina

Copy link
Copy Markdown

@jvsena42 The re-clone question is closed. The cache-hit run is attempt 2 of run 39468042023 at 39555da: it restored the shallow clones, and per @ovitrif's step-log split above, -resolvePackageDependencies took 44 s with no fetch or clone lines. My P1 from 5fedada is resolved by 1772609, so nothing from me is outstanding on this PR.

@ovitrif

ovitrif commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

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 build-local job, which #923 left as it was. No other changes.

On 59385f9 every required check except e2e-status passed, and that one was still queued on the self-hosted runner. The integration run missed the cache because a run on another branch, still on the old workflow, saved an 18 GB macOS-spm-* entry and pushed this one out; that stops as branches pick this change up from master.

@ovitrif
ovitrif requested a review from jvsena42 October 20, 2026 20:02
@ovitrif
ovitrif merged commit a4c8b1b into master Oct 20, 2026
19 checks passed
@ovitrif
ovitrif deleted the ci/faster-integration-tests branch October 20, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants