diff --git a/.github/actions/dart_package/action.yaml b/.github/actions/dart_package/action.yaml index e838c05..b233d63 100644 --- a/.github/actions/dart_package/action.yaml +++ b/.github/actions/dart_package/action.yaml @@ -76,7 +76,7 @@ runs: run: echo "package_name=${PACKAGE_PATH##*/}" >> $GITHUB_OUTPUT - name: Upload Coverage - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: flags: ${{ steps.split.outputs.package_name }} token: ${{ inputs.codecov_token }} diff --git a/.github/actions/flutter_package/action.yaml b/.github/actions/flutter_package/action.yaml index fd90aa9..a1d0fdb 100644 --- a/.github/actions/flutter_package/action.yaml +++ b/.github/actions/flutter_package/action.yaml @@ -82,7 +82,7 @@ runs: run: echo "package_name=${PACKAGE_PATH##*/}" >> $GITHUB_OUTPUT - name: Upload Coverage - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: # We use Codecov's carryforward flags to allow our PR testing to only # run affected packages, but also allow our coverage information from diff --git a/.github/actions/publish_flutter_package/action.yaml b/.github/actions/publish_flutter_package/action.yaml index 06b3b85..281d958 100644 --- a/.github/actions/publish_flutter_package/action.yaml +++ b/.github/actions/publish_flutter_package/action.yaml @@ -10,7 +10,7 @@ runs: using: "composite" steps: - name: 📚 Git Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: 🐦 Setup Flutter uses: subosito/flutter-action@v2 diff --git a/.github/actions/rust_crate/action.yaml b/.github/actions/rust_crate/action.yaml index eaa0318..53dd001 100644 --- a/.github/actions/rust_crate/action.yaml +++ b/.github/actions/rust_crate/action.yaml @@ -41,7 +41,7 @@ runs: run: echo "package_name=${PACKAGE_PATH##*/}" >> $GITHUB_OUTPUT - name: Upload Coverage - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: # We use Codecov's carryforward flags to allow our PR testing to only # run affected packages, but also allow our coverage information from diff --git a/.github/workflows/_shorebird_ci_flutter.yaml b/.github/workflows/_shorebird_ci_flutter.yaml index e1afbae..f3305c1 100644 --- a/.github/workflows/_shorebird_ci_flutter.yaml +++ b/.github/workflows/_shorebird_ci_flutter.yaml @@ -34,7 +34,7 @@ jobs: ci: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: submodules: recursive - name: Setup Flutter @@ -64,7 +64,7 @@ jobs: working-directory: ${{ inputs.package_path }} run: flutter test --coverage - if: inputs.has_unit_tests - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: flags: ${{ inputs.package_name }} working-directory: ${{ inputs.package_path }} diff --git a/.github/workflows/build_patch_artifacts.yaml b/.github/workflows/build_patch_artifacts.yaml index c5e589f..019df22 100644 --- a/.github/workflows/build_patch_artifacts.yaml +++ b/.github/workflows/build_patch_artifacts.yaml @@ -25,7 +25,7 @@ jobs: name: 🦀 Upload Artifacts steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: taiki-e/upload-rust-binary-action@v1 with: bin: "patch" diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml index 862f702..7ffc043 100644 --- a/.github/workflows/main.yaml +++ b/.github/workflows/main.yaml @@ -32,7 +32,7 @@ jobs: steps: - name: 📚 Git Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 - uses: dorny/paths-filter@v4 name: Build Detection @@ -82,7 +82,7 @@ jobs: steps: - name: 📚 Git Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: 🦀 Build ${{ matrix.crate }} uses: ./.github/actions/rust_crate @@ -104,7 +104,7 @@ jobs: steps: - name: 📚 Git Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: submodules: recursive diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 462379b..cc55bf4 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -13,7 +13,7 @@ jobs: runs-on: ubuntu-latest steps: - name: 📚 Git Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: submodules: recursive diff --git a/.github/workflows/shorebird_ci.yaml b/.github/workflows/shorebird_ci.yaml index 790f5e5..6613d9d 100644 --- a/.github/workflows/shorebird_ci.yaml +++ b/.github/workflows/shorebird_ci.yaml @@ -17,7 +17,7 @@ jobs: shorebird_code_push: ${{ steps.filter.outputs.shorebird_code_push }} shorebird_code_push_example: ${{ steps.filter.outputs.shorebird_code_push_example }} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: # Full history so dorny/paths-filter can diff on push events. fetch-depth: 0 @@ -65,7 +65,7 @@ jobs: name: CSpell runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: submodules: recursive - uses: streetsidesoftware/cspell-action@v8 diff --git a/library/Cargo.toml b/library/Cargo.toml index 3205cda..ab3065a 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -73,7 +73,7 @@ oslog = "0.2.0" simple_logger = "5.0.0" [dev-dependencies] -mockall = "0.14.0" +mockall = "0.15.0" mockito = "1.2.0" mock_instant = "0.6.0" # Gives #[serial] attribute for locking all of our shorebird_init diff --git a/library/src/updater.rs b/library/src/updater.rs index e0bf11d..caa3f1d 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -289,7 +289,7 @@ pub fn check_for_downloadable_update(channel: Option<&str>) -> anyhow::Result) -> anyhow::Resul Ok(PatchCheckRequest::new( &config, &state.client_id(), - state.currently_booting_patch().map(|p| p.number), + state.running_patch().map(|p| p.number), )) })?; @@ -4087,3 +4087,110 @@ mod multi_engine_tests { Ok(()) } } + +/// Regression tests asserting the patch-check request's `current_patch_number` +/// carries the running patch, not the boot breadcrumb (`currently_booting_patch`) +/// which `report_launch_success` clears before any patch check fires. Covers +/// both construction sites: `check_for_downloadable_update` and `update`. +#[cfg(test)] +mod patch_check_current_patch_number_tests { + use anyhow::Result; + use serial_test::serial; + use std::sync::atomic::{AtomicI64, Ordering}; + use tempfile::TempDir; + + use crate::{ + check_for_downloadable_update, + network::{ + testing_set_network_hooks, PatchCheckResponse, UNEXPECTED_DOWNLOAD, UNEXPECTED_REPORT, + }, + report_launch_start, report_launch_success, + test_utils::install_fake_patch, + update, + updater::tests::init_for_testing, + with_state, + }; + + /// `current_patch_number` was `None` on the wire (the bug we're guarding). + const FIELD_OMITTED: i64 = -1; + /// The patch-check hook never ran. + const HOOK_NOT_CALLED: i64 = i64::MIN; + + /// Last `current_patch_number` seen by the patch-check hook. Shared by + /// both tests; safe because they are `#[serial]` (only one runs at a + /// time) and `arrange_capturing_hooks` resets it before each check. + static CAPTURED: AtomicI64 = AtomicI64::new(HOOK_NOT_CALLED); + + /// Installs patch 1 and completes a full boot, then asserts the steady + /// state in which patch checks actually run: the boot breadcrumb is + /// cleared, but the process is still running patch 1. Reading + /// `currently_booting_patch` here yields `None` — that is precisely the + /// regression this guards against. + fn boot_patch_one(tmp_dir: &TempDir) -> Result<()> { + init_for_testing(tmp_dir, None); + install_fake_patch(1)?; + report_launch_start()?; + report_launch_success()?; + with_state(|state| { + // Steady state: the boot breadcrumb is cleared, yet patch 1 is + // still the running patch. + assert!(state.currently_booting_patch().is_none()); + assert_eq!(state.running_patch().map(|p| p.number), Some(1)); + Ok(()) + }) + } + + /// Installs network hooks whose patch-check leg records the request's + /// `current_patch_number` into `CAPTURED` (`None` -> `FIELD_OMITTED`) and + /// reports no available update. + fn arrange_capturing_hooks() { + CAPTURED.store(HOOK_NOT_CALLED, Ordering::SeqCst); + testing_set_network_hooks( + |_url, request| { + CAPTURED.store( + request + .current_patch_number + .map(|n| n as i64) + .unwrap_or(FIELD_OMITTED), + Ordering::SeqCst, + ); + Ok(PatchCheckResponse { + patch_available: false, + patch: None, + rolled_back_patch_numbers: None, + }) + }, + // No update is offered, so neither hook should ever fire. + UNEXPECTED_DOWNLOAD, + UNEXPECTED_REPORT, + ); + } + + #[serial] + #[test] + fn check_for_downloadable_update_sends_running_patch() -> Result<()> { + let tmp_dir = TempDir::new().unwrap(); + boot_patch_one(&tmp_dir)?; + arrange_capturing_hooks(); + + check_for_downloadable_update(None)?; + + // 1 = running patch sent; -1 = field omitted (the bug); i64::MIN = hook never ran. + assert_eq!(CAPTURED.load(Ordering::SeqCst), 1); + Ok(()) + } + + #[serial] + #[test] + fn update_sends_running_patch() -> Result<()> { + let tmp_dir = TempDir::new().unwrap(); + boot_patch_one(&tmp_dir)?; + arrange_capturing_hooks(); + + update(None)?; + + // 1 = running patch sent; -1 = field omitted (the bug); i64::MIN = hook never ran. + assert_eq!(CAPTURED.load(Ordering::SeqCst), 1); + Ok(()) + } +}