Skip to content

πŸ›  - Develop fails its own gates: one failing unit test and one clippy errorΒ #163

Description

@jvegaf

Description

Two independent defects make a clean develop (v0.3.0, 757e3cf) fail its own gates. Both are small β€” each is a one-line fix β€” so they can land as two separate PRs.

1. cargo test --features desktop fails: 294 passed, 1 failed, 5 ignored.

failures:
    updater::tests::dev_builds_never_update

thread 'updater::tests::dev_builds_never_update' panicked at src/updater.rs:110:
assertion failed: !accepts("0.3.0-dev", "0.4.0-dev")

The test encodes "a dev build never updates"; accepts_release implements something weaker. It requires the same channel and then plain semver ordering:

// src-tauri/src/updater.rs:44-54
pub fn accepts_release(current: &Version, remote: &Version) -> bool {
    let (current_channel, remote_channel) = (channel_of(current), channel_of(remote));
    if current_channel != remote_channel {
        log::warn!(/* ... */);
        return false;
    }
    remote > current
}

channel_of (updater.rs:31-39) maps -staging.* to Staging, no prerelease to Production, and every other prerelease to ReleaseChannel::Other β€” a shared bucket for -dev and anything else. Two -dev versions therefore sit on the same channel, and 0.4.0-dev > 0.3.0-dev is plain semver-true, so it is accepted while the test says it must not be.

The module doc explains why nobody noticed: Other is "never matched by a published manifest" (updater.rs:16), so the case cannot arise from a real update endpoint β€” the test asserts a stronger invariant than the code implements. Whichever direction is intended, it is one line: add an Other guard in accepts_release, or drop that third assertion and document why.

2. cargo clippy --features desktop -- -D warnings fails, and that is the CI gate.

error: called `Iterator::last` on a `DoubleEndedIterator`; this will needlessly iterate the entire iterator
   --> src/services/device.rs:95:49
    |
 95 |    || target_str.ends_with(device.split('/').last().unwrap_or(""))
    |                                          ^^^^^^^^^^^^^^^^^^------
    = note: `-D clippy::double-ended-iterator-last` implied by `-D warnings`

The rust-lint job runs exactly that command, with the pinned nightly-2026-02-19 toolchain (.github/workflows/ci.lint.yml:99-124), on every push to any branch. So this job is red on develop today and on every branch cut from it, which means the gate carries no signal for anyone.

Why neither was caught by CI. Nothing runs the full suite: the only cargo test in CI is the ignored canary group (ci.canary.yml:109 β€” cargo test --features desktop -- --ignored --test-threads=1 canary_), so dev_builds_never_update never executes. The clippy failure, by contrast, is on CI's path.

Separately worth a look: no workflow declares a pull_request trigger β€” every workflow triggers on push to any branch β€” so a PR opened from a fork does not run these jobs at all.

Both defects were reproduced from a clean checkout of upstream/develop with the repository's pinned toolchain, on Linux.

Open questions

  • Is the dev-build rule an implementation gap or an over-specified test? The channel design (only Production and Staging manifests exist) suggests a dev version can never be offered, which supports the test being over-specified. The test name suggests the guard is missing. Maintainer call.
  • Should the full test suite run in CI? Not required to fix either defect, but it is the reason the first one rotted unnoticed.

Acceptance criteria

  • cargo test --features desktop is green on develop (no failing test).
  • cargo clippy --features desktop -- -D warnings is green on develop with the pinned toolchain.
  • The intended dev-channel behaviour is explicit in the code: either guarded in accepts_release, or documented where the assertion was removed.

Tasks

  • Resolve updater.rs:110 against updater.rs:44-54 β€” one line either way.
  • Use next_back() instead of .last() at src-tauri/src/services/device.rs:95.
  • Re-run both gates and confirm green.
  • Optional, separate: add a cargo test --features desktop job so the suite cannot rot silently again, and consider a pull_request trigger.

Found while verifying an unrelated one-file fix applied on top of upstream/develop. The fix under verification was green; these two failures are pre-existing on the base and are not caused by it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions