From 89ea931ee11abd0754c2c24bf28b57391214f942 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 10 Jul 2026 20:08:31 +0800 Subject: [PATCH] test(ci): strict nextest ci profile with quarantine + flake policy (#4666) test(ci): add strict nextest ci profile with quarantine + flake policy Formalize the existing ecstore-serial-flaky mechanism into a strict CI gate (ci-10, absorbs infra-15; backlog#1149). - .config/nextest.toml: add [profile.ci] with global retries=0 (never mask a new race's first occurrence), fail-fast=false, and JUnit output at target/nextest/ci/junit.xml. Add a quarantine section where flaky tests get retries=2 under the ci profile only; each entry links one OPEN issue. First members are the two backlog#937 ecstore groups (concurrent_resend_same_part_commits_one_generation and store::bucket::tests::bucket_delete_*), which keep their existing ecstore-serial-flaky test-group serialization. Local default profile still never retries. - .github/workflows/ci.yml: run the main test step with --profile ci and upload the JUnit report (if: always(), 3-day retention, run-number in name). The migration-proof step stays on the default profile to avoid clobbering the ci JUnit artifact (its tests are not quarantined). - docs/testing/README.md: new skeleton (owned by backlog#1153 infra-11) holding the flake policy: discover -> open issue within 24h -> quarantine with issue link -> fix or delete within 30 days. AGENTS.md points to it. Refs: rustfs/backlog#1149, rustfs/backlog#937, rustfs/backlog#1155 --- .config/nextest.toml | 54 ++++++++++++++++++++++++++++++++++++++++ .github/workflows/ci.yml | 14 ++++++++++- .gitignore | 2 ++ AGENTS.md | 5 ++++ docs/testing/README.md | 54 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 docs/testing/README.md diff --git a/.config/nextest.toml b/.config/nextest.toml index eff3034cd..4b435fe07 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -17,9 +17,63 @@ # nextest executes each test in its own process, where the in-process # serial_test mutex has no effect. A nextest test-group with max-threads = 1 is # the mechanism that actually serializes across nextest's process boundary. +# +# --------------------------------------------------------------------------- +# Profiles +# --------------------------------------------------------------------------- +# The `default` profile is what local `cargo nextest run` uses. It NEVER +# retries: a red test locally means a real failure to investigate, not noise to +# paper over. The `ci` profile (below) is the strict CI gate: global +# retries = 0 so a new race's first occurrence is never masked, plus a +# narrowly-scoped quarantine list (retries = 2) for tests with a tracked OPEN +# flake issue. Flake policy lives in docs/testing/README.md. + [test-groups] ecstore-serial-flaky = { max-threads = 1 } +# --- default profile (local): serialize the flaky groups, never retry -------- [[profile.default.overrides]] filter = 'package(rustfs-ecstore) & (test(concurrent_resend_same_part_commits_one_generation) | test(/^store::bucket::tests::bucket_delete_(mark_delete_marks|purge_removes|default_s3_delete)/))' test-group = 'ecstore-serial-flaky' + +# --------------------------------------------------------------------------- +# ci profile — the strict CI gate (ci.yml `cargo nextest run --profile ci`) +# --------------------------------------------------------------------------- +[profile.ci] +# Strict: a new race must fail on its first occurrence, never be retried away. +retries = 0 +# Report every failure in one run instead of bailing on the first. +fail-fast = false + +[profile.ci.junit] +# Emitted to target/nextest/ci/junit.xml; uploaded as a CI artifact. +# Tests that pass only after a quarantine retry are marked `flaky` here — that +# marker is the observable signal the flake policy is built around. +path = "junit.xml" + +# =========================================================================== +# QUARANTINE — flaky tests granted retries = 2 under the ci profile ONLY. +# +# RULES (enforced by review, see docs/testing/README.md): +# * Every entry MUST link exactly one OPEN issue tracking the flake. +# * An entry stays until the issue is fixed (test made robust) or the test is +# deleted — 30-day policy. No entry may exist without a live issue link. +# +# Each entry also re-declares the `ecstore-serial-flaky` test-group so the +# serialization holds under the ci profile (nextest evaluates a named +# profile's own overrides list, not the default profile's). +# =========================================================================== + +# QUARANTINE: OPEN backlog#937 — concurrent_resend lock-acquire deadline flakes +# under saturated disk I/O in the full parallel suite. +[[profile.ci.overrides]] +filter = 'package(rustfs-ecstore) & test(concurrent_resend_same_part_commits_one_generation)' +test-group = 'ecstore-serial-flaky' +retries = 2 + +# QUARANTINE: OPEN backlog#937 — store::bucket::tests::bucket_delete_* race +# make_bucket into InsufficientWriteQuorum via shared global state under load. +[[profile.ci.overrides]] +filter = 'package(rustfs-ecstore) & test(/^store::bucket::tests::bucket_delete_(mark_delete_marks|purge_removes|default_s3_delete)/)' +test-group = 'ecstore-serial-flaky' +retries = 2 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c57d8d4e..b55082482 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -155,12 +155,24 @@ jobs: - name: Run tests run: | - cargo nextest run --all --exclude e2e_test + cargo nextest run --profile ci --all --exclude e2e_test cargo test --all --doc + - name: Upload test junit report + if: always() + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6 + with: + name: junit-test-and-lint-${{ github.run_number }} + path: target/nextest/ci/junit.xml + retention-days: 3 + if-no-files-found: ignore + # Explicit gate for migration-critical suites. These tests already ran in # the full nextest pass above; a single filtered nextest invocation keeps # the named gate without rebuilding or re-running them one package at a time. + # Kept on the default profile (no --profile ci): a second --profile ci run + # would clobber target/nextest/ci/junit.xml, and none of these tests are + # quarantined so they gain nothing from the ci profile's retry overrides. - name: Run rebalance/decommission migration proofs run: | cargo nextest run -p rustfs-ecstore --lib \ diff --git a/.gitignore b/.gitignore index e8e079c23..f22ceed7e 100644 --- a/.gitignore +++ b/.gitignore @@ -55,6 +55,8 @@ docs/* !docs/architecture/** !docs/operations/ !docs/operations/** +!docs/testing/ +!docs/testing/** !docs/superpowers/ !docs/superpowers/** docs/heal-scanner-logging-governance.md diff --git a/AGENTS.md b/AGENTS.md index 5d185e942..9cc049965 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,6 +108,11 @@ list, suppress a lint with `#[allow]`, mark a failing test `#[ignore]`, or delete or relax a failing assertion to get green. If a check itself is wrong, change it deliberately and state the rationale in the PR. +For flaky tests, do not paper over them with retries. Follow the flake policy +in [docs/testing/README.md](docs/testing/README.md) (open an issue within 24h, +quarantine with an issue link, fix or delete within 30 days); the local +`default` nextest profile never retries. + ## Adversarial Validation (Default On) Every non-exempt output (see Risk tiers) — code change, bug fix, or diff --git a/docs/testing/README.md b/docs/testing/README.md new file mode 100644 index 000000000..e4ad04829 --- /dev/null +++ b/docs/testing/README.md @@ -0,0 +1,54 @@ +# RustFS Testing + +> **Owner: backlog#1153 (infra-11).** This file is the authoritative home for +> test taxonomy, naming conventions, and serial/quarantine rules. It is +> currently a **skeleton** — infra-11 fills in the remaining sections. The +> event × budget × required matrix lives in `docs/testing/ci-gates.md` +> (ci-15); this file links to it rather than duplicating counts. + +## Test taxonomy + +_TODO (infra-11): unit / e2e-smoke / e2e-full / e2e-nightly / protocols / +s3-tests / fuzz / perf, with naming conventions._ + +## Serial groups & CI profiles + +_TODO (infra-11): document the `[test-groups]` mechanism and the `default` vs +`ci` nextest profiles. See `.config/nextest.toml`._ + +## Flake policy + +A flaky test is one that fails non-deterministically without a corresponding +code change. Flakes erode trust in the gate and block tightening required +checks, so they are handled on a strict, time-boxed loop. + +**Retry semantics (source of truth: `.config/nextest.toml`):** + +- The **local `default` profile never retries.** A red test on your machine is + a real failure to investigate, not noise to paper over. +- The **CI `ci` profile runs with global `retries = 0`.** A new race must fail + on its first occurrence so the first crime scene is never masked. +- Only tests on the **quarantine list** get `retries = 2`, and only under the + `ci` profile. Each quarantine entry MUST link exactly one OPEN issue. +- **JUnit flaky markers are the observable.** A quarantined test that passes + only after a retry is marked `flaky` in `target/nextest/ci/junit.xml` + (uploaded as a CI artifact). That marker — not a green check — is how we see + a flake is still live. + +**Lifecycle of a flake:** + +1. **Discover** — a test fails non-deterministically (CI or local), or shows a + `flaky` marker in the JUnit report. +2. **Open an issue within 24h** — file/track an issue describing the flake + (symptom, suspected cause, affected suite). No silent re-runs. +3. **Quarantine** — add the test to the quarantine override block in + `.config/nextest.toml` with a comment linking that OPEN issue. This grants + `retries = 2` under CI so the flake stops reddening unrelated PRs, while the + `flaky` marker keeps it visible. +4. **Fix or delete within 30 days** — make the test robust (then remove the + quarantine entry) or delete the test. A quarantine entry may not outlive its + fix window; an entry without a live OPEN issue link is a policy violation. + +First quarantine members: the two backlog#937 ecstore groups +(`concurrent_resend_same_part_commits_one_generation` and +`store::bucket::tests::bucket_delete_*`).