ci: make the live job honest about what it ran and skipped

Three holes in the live job. (1) It ran -p x-media only, on the comment that everything network-gated lives there — false: xmedia-bot carries two #[ignore]d live tests (repair refetch, text-only link) that CI never executed; both crates run now, still behind the 'live' name filter, and the bot crate's tests need no token (their sends go through MockSender). (2) BILIBILI_COOKIE was never passed, so the bilibili live tests always hit the risk-control early return. (3) That early return — like the pixiv download test's token gate — printed an indistinguishable 'skipping:' line that --show-output never showed, so with continue-on-error a fully-skipped weekly run read exactly like full coverage; skips now print a 'SKIP ' prefix, the steps tee with --show-output, and a summary step lists every skip in the run summary plus a workflow warning. The redundant 'Run token-gated tests' step (the whole non-ignored suite, a rerun of the test job) goes away with the comment that justified it.
This commit is contained in:
2026-09-24 03:45:17 +08:00
parent a49bb1d1af
commit 7b50ad82b0
3 changed files with 27 additions and 12 deletions
+22 -8
View File
@@ -140,15 +140,29 @@ jobs:
env:
PIXIV_REFRESH_TOKEN: ${{ secrets.PIXIV_REFRESH_TOKEN }}
TWITTER_AUTH_TOKEN: ${{ secrets.TWITTER_AUTH_TOKEN }}
BILIBILI_COOKIE: ${{ secrets.BILIBILI_COOKIE }}
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2
# Everything network- or secret-gated lives in x-media, and the bot
# crate's suite (MockSender + tempdir stores, no network) already ran in
# the `test` job — rebuilding it here bought nothing.
- name: Run token-gated tests
run: cargo test -p x-media --locked
# The live-network tests, by the "live" name filter (all #[ignore]d).
- name: Run live-network tests
run: cargo test -p x-media --locked -- --ignored live
# All #[ignore]d live tests, by the "live" name filter, in both crates:
# the bot crate has two of its own (repair refetch, text-only link)
# whose fetches are network-bound but whose sends go through MockSender.
# --show-output keeps each test's stdout — the SKIP lines printed by a
# skipped test are what the summary step below greps, so a skip stays
# distinguishable from a pass.
- name: Run live-network tests (x-media)
run: cargo test -p x-media --locked -- --ignored live --show-output 2>&1 | tee live-x-media.log
- name: Run live-network tests (xmedia-bot)
run: cargo test -p xmedia-bot --locked -- --ignored live --show-output 2>&1 | tee live-bot.log
# A green live run must not be able to mean "nothing actually ran"
# (pixiv without its secret, bilibili risk-controlling the runner IP):
# collect every SKIP line into the run summary.
- name: Surface skipped live tests
if: always()
run: |
skips=$(grep -h '^SKIP ' live-*.log 2>/dev/null || true)
if [ -n "$skips" ]; then
echo "::warning::live job skipped tests, see the job summary"
{ echo "### Live tests skipped"; echo "$skips" | sed 's/^/- /'; } >> "$GITHUB_STEP_SUMMARY"
fi
+1 -1
View File
@@ -109,6 +109,6 @@ Docker: `docker build -t tgxmb .` then `docker run --rm -d --name tgxmb --env-fi
- No mocking framework anywhere (no mockito/wiremock/mockall). Conventions: pure-function units (regex parsing, serde round-trips, chunking, retry math) tested synchronously; async tests use real dependencies — file-backed SQLite via `tempfile` (`queue.rs::new_queue()` helper), live network fetches. Tests that must go through a **real `Bot`** (its URL/multipart building, the per-chat limiter and the bot-wide budget) talk to a stand-in API instead (`media_sender::test_support::fake_api::FakeApi`, a `tokio` TCP listener that records every call and answers the smallest result each method needs — teloxide keys methods by payload type, so the recorded name is `SendMediaGroup`, not `sendMediaGroup`): a media group, the edit-before-forward prompt through the real callback path, and `handlers::handle_message` (the context-taking body of `message_handler`, split out for exactly this).
- Live-network tests exist in `site/twitter/interface.rs` (5), `site/bsky/interface.rs` (1), `site/misskey/interface.rs` (1), `site/bilibili/interface.rs` (5), `site/pixiv/api.rs` (1); `photo.rs` adds one `#[ignore = "heavy: …"]` test. `site/download.rs`'s pixiv download test (`live_download_media_pixiv_original_with_referer`) is gated **both ways** — `#[ignore = "live network: …"]` *and* an early return without `PIXIV_REFRESH_TOKEN` — so a local `cargo test --workspace` stays fully offline and the pixiv CDN flake surfaces only in the live job. `disabled_site_is_reported_not_ignored` (same file) is gated the other way round: it asserts `fetch` answers `FetchError::Disabled { site: "pixiv" }` for a pixiv link and early-returns when `PIXIV_REFRESH_TOKEN` **is** set (the site is then enabled). Test gating convention (enforced by `.github/workflows/ci.yml`): pure unit tests always run; live-network tests carry `#[ignore = "live network: ..."]` (run via `cargo test --workspace -- --ignored live`); token-gated pixiv tests early-return when `PIXIV_REFRESH_TOKEN` is absent **or empty** (an unset GitHub secret arrives as `""` — `is_err()` alone would run them tokenless and fail), and the bilibili live tests early-return when the API answers risk control (`-352`, which bilibili applies per IP by request volume). Run the full offline suite with `cargo test --workspace`.
- Fixtures are inline `serde_json::json!` builder fns (`fixture()`, `thread_json()`, `illust_json()`), not files. The shared `CLIENT` sets `pool_max_idle_per_host(0)` under `#[cfg(test)]` to avoid cross-runtime `DispatchGone`.
- **CI** — `.github/workflows/ci.yml` (actions pinned to commit SHAs, `--locked` on every cargo invocation, `concurrency` cancels superseded runs, `RUST_BACKTRACE=1`) runs (behind a `changes` gate job, so a push/PR whose entire diff is markdown skips it instead of burning four minutes on nothing) `cargo fmt --check` + `cargo clippy --workspace --all-targets --locked -- -D warnings` + `cargo test --workspace --locked` + a release-profile `cargo build --release --locked` + an `actions-rust-lang/audit` dependency-vulnerability gate (offline, no secrets, on every push/PR) and a `live` job (schedule/manual/tag only, `-p x-media` since every network/secret-gated test lives there, `continue-on-error`) for the `#[ignore]`d live + token tests. `.github/workflows/docker.yml` builds and pushes the image on master/tag and runs a **build-only check on pull requests touching the build inputs**; its `should-build` gate skips a branch push that is already tagged (`git tag --points-at` — the tag run builds it, so push both refs together) or that touched no build input at all, while a tag push always builds (`Dockerfile`, entrypoint, manifests, `.dockerignore`); a release tag must match both crate versions or the build stops, and `FFMPEG_URL`/`FFMPEG_SHA256` are taken from repository variables when set (a release can pin an exact ffmpeg build). `.github/dependabot.yml` keeps crates, the pinned actions and the Docker base images current.
- **CI** — `.github/workflows/ci.yml` (actions pinned to commit SHAs, `--locked` on every cargo invocation, `concurrency` cancels superseded runs, `RUST_BACKTRACE=1`) runs (behind a `changes` gate job, so a push/PR whose entire diff is markdown skips it instead of burning four minutes on nothing) `cargo fmt --check` + `cargo clippy --workspace --all-targets --locked -- -D warnings` + `cargo test --workspace --locked` + a release-profile `cargo build --release --locked` + an `actions-rust-lang/audit` dependency-vulnerability gate (offline, no secrets, on every push/PR) and a `live` job (schedule/manual/tag only, `continue-on-error`) that runs the `#[ignore]`d `live` tests in **both** crates — `PIXIV_REFRESH_TOKEN`/`TWITTER_AUTH_TOKEN`/`BILIBILI_COOKIE` pass through as secrets, and a summary step lists every `SKIP` a test printed so a green live run cannot mean zero coverage. `.github/workflows/docker.yml` builds and pushes the image on master/tag and runs a **build-only check on pull requests touching the build inputs**; its `should-build` gate skips a branch push that is already tagged (`git tag --points-at` — the tag run builds it, so push both refs together) or that touched no build input at all, while a tag push always builds (`Dockerfile`, entrypoint, manifests, `.dockerignore`); a release tag must match both crate versions or the build stops, and `FFMPEG_URL`/`FFMPEG_SHA256` are taken from repository variables when set (a release can pin an exact ffmpeg build). `.github/dependabot.yml` keeps crates, the pinned actions and the Docker base images current.
- Untested and hard to test without a mock seam: `config.rs`, `handlers/statics.rs`; `db.rs` is covered for the migration chain but not for pool behaviour under contention; `main.rs` is covered where it was split out (`periodic_sweep`, `sweep_temp_dir`) but not for startup/shutdown or its `dptree` branch tree (the handlers themselves are, through the stand-in API); in `x-media`: `media.rs`, `lib.rs`, all `model.rs`. The `commands.rs` *executor* needs a real `Bot` (only its pure report builder is tested). Everything else — `handlers/{mod,callback,inline,urls}.rs`, `send/*`, `ctx.rs`, `state.rs`, `queue.rs`, `link_cache.rs`, `rate_limit.rs` — is driven through `TestStores`/`ctx::test_support` and the scripted `MockSender`.
- No coverage tracking.
@@ -1007,13 +1007,14 @@ mod tests {
/// Fetches a live dynamic, skipping the assertion when bilibili
/// risk-controls this IP (the site blocks datacenter/over-used addresses
/// with `-352` regardless of cookies — a real failure would surface as a
/// parse error or a not-found instead). Mirrors the token-gated pixiv
/// tests' "skipping: …" convention.
/// parse error or a not-found instead). Mirrors the pixiv download
/// test's `SKIP …` convention — CI's live job greps that prefix to list
/// the skips in the run summary instead of showing a silently green run.
async fn live_fetch(url: &str) -> Option<Fetched> {
match fetch_from_url(url).await {
Ok(fetched) => Some(fetched),
Err(e) if e.to_string().contains("risk control") => {
eprintln!("skipping: {e}");
eprintln!("SKIP (bilibili risk control): {e}");
None
}
Err(e) => panic!("{e}"),