Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 34 additions & 136 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ env:
CARGO_INCREMENTAL: 0
# Keep file/line panic locations while cutting codegen, link time (Windows
# especially) and cache size. Dependency debug info is already off in
# Cargo.toml; this trims the workspace crates and test binaries too.
# Cargo.toml; this trims the workspace crates too. The test profile keeps no
# debug info at all: a test binary is the whole GPUI stack, and a panic
# message and its `assert_eq!` output carry the location a CI failure needs,
# while a backtrace's line numbers rarely decide the diagnosis.
CARGO_PROFILE_DEV_DEBUG: line-tables-only
CARGO_PROFILE_TEST_DEBUG: line-tables-only
CARGO_PROFILE_TEST_DEBUG: "0"

# A newer push to the same ref replaces an in-flight run: before this,
# superseded merges kept burning 10-25 minutes each.
Expand Down Expand Up @@ -53,7 +56,13 @@ jobs:
# environment hash varies across runner images. Put the lockfile
# hash in the prefix: it rotates when dependencies change and stays
# stable across runners.
prefix-key: v0-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml', 'Cargo.toml', 'crates/*/Cargo.toml') }}
#
# Only Cargo.lock and the toolchain: hashing the manifests too made
# every release-please version bump a cache miss, and the release run
# compiled all 850 crates cold on every OS (observed: Windows clippy
# 4m24 against 57s warm). A cache missing a newly added artifact is
# not a correctness problem -- cargo compiles that delta.
prefix-key: v1-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }}
add-rust-environment-hash-key: "false"
cache-on-failure: "true"
- name: Install system dependencies
Expand All @@ -64,9 +73,16 @@ jobs:
libfontconfig-dev libfreetype-dev libwayland-dev libxkbcommon-dev \
libxkbcommon-x11-dev libx11-xcb-dev libxcb1-dev libvulkan1 \
libssl-dev libzstd-dev libdbus-1-dev
# Pinned by commit SHA, version tag in the comment, like the actions
# above. nextest runs test binaries concurrently instead of one at a
# time, which is where this suite's wall clock was going: `cargo test`
# took 2m51 here, 60s of it inside one real-time bandwidth binary.
- uses: taiki-e/install-action@5488ac5a951645ce95062214902ac4b6bc1a1037 # v2.87.32
with:
tool: cargo-nextest@0.9.150
- run: cargo fmt --all -- --check
- run: cargo clippy --locked --workspace --all-targets -- -D warnings
- run: cargo test --locked --workspace
- run: cargo nextest run --locked --workspace

windows:
name: Windows
Expand All @@ -84,13 +100,16 @@ jobs:
# `key:` is ignored with `shared-key:`. lld-link was tried here and
# reverted: the linker is a global fingerprint input, so switching it
# rebuilt all 526 crates every run and never warmed the cache.
prefix-key: v0-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml', 'Cargo.toml', 'crates/*/Cargo.toml') }}
prefix-key: v1-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }}
add-rust-environment-hash-key: "false"
cache-on-failure: "true"
# fmt is platform-independent and already checked on Linux; running it
# here only paid for a toolchain component and a fresh checkout.
- uses: taiki-e/install-action@5488ac5a951645ce95062214902ac4b6bc1a1037 # v2.87.32
with:
tool: cargo-nextest@0.9.150
- run: cargo clippy --locked --workspace --all-targets -- -D warnings
- run: cargo test --locked --workspace
- run: cargo nextest run --locked --workspace

# The only place the macOS platform arms are compiled and exercised: the
# maintainer's Linux host cannot build them (aws-lc-sys needs a macOS C
Expand Down Expand Up @@ -118,14 +137,17 @@ jobs:
- uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
with:
shared-key: macos
prefix-key: v0-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml', 'Cargo.toml', 'crates/*/Cargo.toml') }}
prefix-key: v1-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }}
add-rust-environment-hash-key: "false"
cache-on-failure: "true"
- uses: taiki-e/install-action@5488ac5a951645ce95062214902ac4b6bc1a1037 # v2.87.32
with:
tool: cargo-nextest@0.9.150
- run: cargo clippy --locked --workspace --all-targets -- -D warnings
# --no-fail-fast: one macOS-only failure must not hide the next test
# binary's. This job is the only place those tests run and each cold run
# costs minutes.
- run: cargo test --locked --workspace --no-fail-fast
# nextest runs every test whatever an earlier one did, which replaces the
# --no-fail-fast this job needed: one macOS-only failure must not hide
# the next one, and this job is the only place those tests run.
- run: cargo nextest run --locked --workspace

windows-gui-subsystem:
name: Windows GUI subsystem (release PE)
Expand All @@ -144,7 +166,7 @@ jobs:
# the other. Separate keys also keep the release profile out of the
# debug cache the critical-path job restores.
shared-key: windows-release
prefix-key: v0-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml', 'Cargo.toml', 'crates/*/Cargo.toml') }}
prefix-key: v1-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml') }}
add-rust-environment-hash-key: "false"
cache-on-failure: "true"
# #28: the app is a tray process, so the release binary must be a GUI
Expand Down Expand Up @@ -214,127 +236,3 @@ jobs:
throw "$exe has no icon resource group 1; GPUI's window class icon and the shell icon would be blank"
}
"icon resource group 1 present"

linux-gui-smoke:
name: Linux GUI smoke (Xvfb + lavapipe)
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
- name: Install Rust 1.99.0
run: |
rustup toolchain install 1.99.0 --profile minimal --component rustfmt --component clippy
rustup default 1.99.0
- uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
with:
# Shared with the Linux job: rust-cache persists dependencies only, so
# this job restores the same dependency artifacts instead of
# compiling them a third time. It never saves: the Linux job owns the
# key and its cache covers the test targets too, while a bin-only save
# here finished first and starved the Linux job of dev-dependencies.
# The prefix must match the Linux job's exactly to share the cache.
shared-key: linux
prefix-key: v0-rust-${{ hashFiles('Cargo.lock', 'rust-toolchain.toml', 'Cargo.toml', 'crates/*/Cargo.toml') }}
add-rust-environment-hash-key: "false"
save-if: "false"
cache-on-failure: "true"
- name: Install system dependencies
run: |
sudo apt-get update
sudo apt-get install -y \
gcc g++ clang pkg-config \
libfontconfig-dev libfreetype-dev libwayland-dev libxkbcommon-dev \
libxkbcommon-x11-dev libx11-xcb-dev libxcb1-dev \
libvulkan1 mesa-vulkan-drivers libdbus-1-dev \
xvfb xauth dbus-daemon imagemagick x11-utils
- run: cargo build --locked --bin immich-sync
- name: Capture a real frame
run: |
set -euo pipefail
# The ICD file name depends on the mesa version (lvp_icd.json vs lvp_icd.x86_64.json).
ICD=$(ls /usr/share/vulkan/icd.d/lvp_icd*.json 2>/dev/null | head -1) || ICD=""
if [ -z "$ICD" ]; then
echo "no lavapipe ICD file under /usr/share/vulkan/icd.d" >&2
exit 1
fi
echo "lavapipe ICD=$ICD"
export VK_ICD_FILENAMES="$ICD"
export VK_DRIVER_FILES="$ICD"
export LIBGL_ALWAYS_SOFTWARE=1
unset WAYLAND_DISPLAY
dbus-run-session -- xvfb-run -a -s "-screen 0 1280x800x24" bash -c '
set -euo pipefail
# Give this run its own log and state directory: a log left by an
# earlier run would satisfy the started poll below before this app
# ever starts, and a stale state database would hand off instead.
export XDG_DATA_HOME="$PWD/xdg-data"
target/debug/immich-sync >shot.png.log 2>&1 &
APP=$!
# The app logs here, not to stdout.
LOG_DIR="${XDG_DATA_HOME:-$HOME/.local/share}/immich-sync"
started() { grep -qs "immich-sync started" "$LOG_DIR"/immich-sync.log* 2>/dev/null; }
alive() { kill -0 $APP 2>/dev/null; }
# Wait for the client instead of sleeping a fixed 10 s: opening the
# store made startup slower, and a fixed budget captured a black
# root while the app was still booting.
START_TIMEOUT=60
waited=$SECONDS
until started; do
alive || { echo "the app exited during startup:" >&2; cat shot.png.log >&2; exit 1; }
if [ $((SECONDS - waited)) -ge $START_TIMEOUT ]; then
echo "timeout: no immich-sync started line in $LOG_DIR after ${START_TIMEOUT}s" >&2
cat shot.png.log >&2
kill $APP 2>/dev/null || true
exit 1
fi
sleep 1
done
echo "app started after $((SECONDS - waited))s"
kill -0 $APP # the app survived startup
# That line is logged before the window opens, so wait for a drawn
# frame too: a bare root captures as one flat colour.
PAINT_TIMEOUT=60
waited=$SECONDS
while :; do
alive || { echo "the app exited before it painted:" >&2; cat shot.png.log >&2; exit 1; }
# Bound the capture: a wedged import must not block the loop past
# the deadline below.
timeout 30 import -window root shot.png
colors=$(identify -format %k shot.png)
if [ "$colors" -gt 1 ]; then break; fi
if [ $((SECONDS - waited)) -ge $PAINT_TIMEOUT ]; then
echo "timeout: the frame is still one flat colour after ${PAINT_TIMEOUT}s (colors=$colors)" >&2
cat shot.png.log >&2
kill $APP 2>/dev/null || true
exit 1
fi
sleep 1
done
echo "frame painted after $((SECONDS - waited))s"
target/debug/immich-sync # second launch: non-primary, exits 0 under set -e
sleep 2
kill -0 $APP # the handoff did not stop the primary
kill $APP 2>/dev/null || true
wait $APP 2>/dev/null || true
cat shot.png.log
echo "--- app log directory: $LOG_DIR"
ls -la "$LOG_DIR" 2>/dev/null || true
newest=$(ls -t "$LOG_DIR"/immich-sync.log* 2>/dev/null | head -1 || true)
if [ -n "$newest" ]; then
echo "--- app log: $newest"
cat "$newest"
else
echo "--- no app log file found"
fi
'
- name: Assert the frame is not blank
run: |
set -euo pipefail
identify -format "geom=%wx%h colors=%k\n" shot.png
colors=$(identify -format %k shot.png)
mean=$(convert shot.png -colorspace gray -format '%[fx:mean]' info:)
echo "screenshot gray mean=$mean"
# The colour test is what rejects a flat frame whatever its
# brightness: a uniform grey frame is colors=1 with mean~0.5, so the
# mean test alone would pass it. Together they reject a bare root.
test "$colors" -gt 1
test "$mean" != "0"
49 changes: 41 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,23 @@ all three; the Windows and macOS jobs run clippy and the tests:
```sh
cargo fmt --all -- --check
cargo clippy --workspace --all-targets -- -D warnings
cargo test --workspace
cargo nextest run --workspace
```

Plus a headless Linux smoke that launches the real binary under Xvfb with
lavapipe and proves the captured frame is not blank. UI changes should keep the
headless GPUI tests green; `cargo test -p immich-sync --test dashboard`,
`--test logs_ui`, and the other `tests/*_ui.rs` files cover the surfaces.
`cargo nextest` (`cargo install --locked cargo-nextest`; CI pins 0.9.150) runs
test *binaries* concurrently instead of one at a time. That was where the gate's
wall clock went: `cargo test --workspace` took 2m51 on the maintainer's box.
nextest takes it to about 47s. Each crate still has one test binary:
`crates/*/tests/it/main.rs` holds the old per-file tests as modules, so a filter
is `cargo nextest run -p immich-sync -E 'test(ui)'` rather than `--test logs_ui`.

No CI job launches the binary. The Linux GUI smoke that used to run it under
Xvfb with lavapipe and assert the captured frame was not blank is gone: it cost
a job and a distro-package list to prove a frame that the headless GPUI tests
already cover in pieces. So a change that touches rendering, startup, the tray,
or the single-instance handoff should keep the headless GPUI tests green — the
`*_ui` modules under `crates/app/tests/it/` cover the surfaces — and be checked
against the capture recipe below on a real machine.

`--all-features` is invalid here: the three `gpui-tray` backends are mutually
exclusive.
Expand All @@ -81,12 +91,35 @@ Some tests are wall-clock budgets rather than progress-bounded waits:
tests in `engine.rs`,
`a_server_that_stops_answering_produces_backoff_not_a_hot_loop`, the pacing
test in `limits.rs`, and the port race in `connection.rs`. In
`crates/app/tests/engine_model.rs`,
`crates/app/tests/it/engine_model.rs`,
`a_click_on_a_running_engine_starts_a_scan` is one too: it hashes a 64 MiB
fixture so the scan outlives the 250 ms status poll that carries the scan flag,
and `EngineModel`'s surface has no deterministic barrier. They can starve under
heavy parallel load; run one alone before treating a failure as a regression.

Concurrency makes the ones that read the clock twice flaky, so a timing test
reads it once: `BandwidthLimit`'s unit tests pass one `Instant` to `set_idle_at`,
`debit_at` and `wait_for_at` instead of letting a descheduled thread refill the
bucket between two `Instant::now()` calls (nextest's parallelism is what exposed
that). Do the same for a new timing assertion rather than widening its bounds.

A test that proves a *window* fires must not pay that window in wall clock.
Shrink it on the client instead of on the clock:
`ImmichClient::with_window(Duration::from_millis(200))` runs the real guard over
a real socket, and `limits.rs` then asserts the arithmetic it actually wants
(one window after the last byte moved, at most two for a body blocked
mid-flight). The production value is asserted once, by
`the_production_window_is_the_specified_thirty_seconds`; a test that re-measures
30 s is measuring the constant, not the guard. Do not reach for
`tokio::time::pause()` to do this: auto-advance cannot tell a parked runtime
waiting on a socket from one waiting on a timer, so a real round-trip races the
next deadline and fails as `TimedOut` (tried; the login handshake failed).

Some tests stay on the real clock because the wire is the subject: the pacing
tests in `limits.rs` cost `bytes / rate` and are irreducible, and the scan and
100-file tests in `sync-core` cost real hashing, uploads and fsyncs. Shrink
those with a smaller fixture, never with a fake clock.

## Where the design lives

| What | Where |
Expand All @@ -100,7 +133,7 @@ heavy parallel load; run one alone before treating a failure as a regression.
## Screenshots

The dashboard image in the README is a real frame from the app, seeded with
`crates/app/tests/screenshot_seed.rs`. Give the capture its own XDG directories
`crates/app/tests/it/screenshot_seed.rs`. Give the capture its own XDG directories
so it cannot touch your real state:

```sh
Expand All @@ -110,7 +143,7 @@ mkdir -p "$XDG_CONFIG_HOME" "$XDG_DATA_HOME/immich-sync"

IMMICH_SYNC_CAPTURE_DB="$XDG_DATA_HOME/immich-sync/state.db" \
IMMICH_SYNC_CAPTURE_ROOT="$XDG_DATA_HOME/capture" \
cargo test -p immich-sync --test screenshot_seed -- --ignored
cargo nextest run -p immich-sync -E 'test(screenshot_seed)' --run-ignored all
```

Then launch the binary with the same environment under Xvfb, with lavapipe for
Expand Down
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/app/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//!
//! Redaction is a property of the recorded types: `Secret` and `SessionToken`
//! redact their `Debug` output, and no event formats `Secret::expose`.
//! `crates/app/tests/log_redaction.rs` greps the real file after real sign-ins
//! `crates/app/tests/it/log_redaction.rs` greps the real file after real sign-ins
//! to prove it.

use std::fs;
Expand Down Expand Up @@ -120,7 +120,7 @@ impl LogLevel {
/// exercise the pruning the appender runs when it opens the directory.
/// `tracing-appender` reads its own clock, so the prune that runs at a timed
/// rollover uses the same code path but cannot be triggered deterministically;
/// see `crates/app/tests/logging.rs`.
/// see `crates/app/tests/it/logging.rs`.
#[derive(Clone, Debug)]
pub struct LogPolicy {
/// When a new log file starts.
Expand Down
2 changes: 1 addition & 1 deletion crates/app/src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub fn main_window_options(cx: &App) -> WindowOptions {
// width the widest row of any surface still fits in, the Dashboard's
// header, whose title block beside the Sync now and Pause buttons needs
// 509 px; 480 px is the height the Logs surface's cards keep their
// floors at in their narrowest wrap (#54). `tests/shell.rs` stands at
// floors at in their narrowest wrap (#54). `tests/it/shell.rs` stands at
// exactly this size, so a layout that outgrows it fails there.
window_min_size: Some(size(px(520.), px(480.))),
..Default::default()
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! the whole dashboard. The rendered-view test opens a real headless window and
//! reads native accessibility properties and layout bounds.

mod common;
use super::common;

use std::collections::BTreeSet;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! cannot read pixels, so "readable in both themes" is checked by rendering
//! under both and reading the same accessibility properties.

mod common;
use super::common;

use common::{contrast, mock_server, press_reaches};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! The app-scoped engine model's start handoff (SPEC R8.7).

mod common;
use super::common;

use common::mock_server;

Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
//! state database, the real engine model, and the view's own click and keyboard
//! handling.

mod common;
use super::common;

use std::path::Path;
use std::sync::Arc;
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! echoes the submitted password in its error body, so a server-supplied
//! message cannot slip a secret into the log either.

mod common;
use super::common;

use common::mock_server;

Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
//! properties; the colour tokens themselves are covered by the WCAG test in
//! `tests/dashboard.rs`.

mod common;
use super::common;

use std::fs;
use std::path::{Path, PathBuf};
Expand Down
Loading
Loading