mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
test: give the cross-stack contracts a named tier (RL-11)
`Client/tests/unit/admin-static-channel-perms.test.ts` reads and executes
`Server/admin/static/index.html`. Filed under `tests/unit`, nothing about its
location or name said it locks a server-owned artifact, so a Go developer
editing the admin SPA got a red check called "Client Unit Tests" with no clue
why.
The register describes this as one file. It is not, and the measured set does
not match the description in either direction:
- Client -> Server: exactly ONE test crosses by filesystem read, not two.
`main-page.test.ts` was named in the plan but only carries a prose comment
citing `Server/admin/update_handlers.go:181` at line 1046 — no read, no
import, nothing to move.
- Server -> Client: the four tests the plan named do not cross.
`waf_test.go`/`waf_crs_test.go` set a `User-Agent: OwnCordClient/1.0`
literal that appears nowhere under `Client/`; `ws_integration_test.go:289`
and `sanitize_content_fuzz_test.go:46` are comments. The real crossing is
one the register never named: `Server/updater/updater_test.go:630` does
`os.ReadFile` on `Client/src-tauri/tauri.conf.json`.
The obvious fixes are both wrong. Moving the invariant "to the owning server
test" cannot work: `Server/go.mod` carries no JavaScript engine (no goja,
otto, v8go, quickjs, rogchap, duktape), so a Go port could only assert at the
text level like `admin/perm_grid_test.go` does — and that is not a
substitute. Flipping the guard at `admin/static/index.html:1182` to
`targetIsTouchedRole=false` reintroduces OC-0154 in full while leaving every
greppable identifier intact, so a text-level test passes on a broken file.
Relocating it to the e2e admin journey is worse: that job is
`continue-on-error: true` and deliberately unpinned ("requiring it is
theatre" — `docs/plans/b0-dev-branch-protection.sh`), so it would convert a
blocking, pinned gate into one that is green regardless. And the journey does
not cover the invariant today: `grep -Eic "perm|access|role|override|matrix"`
over its 142 lines returns 0, so the "if e2e already covers it, delete"
branch never fires.
Done — one tier, applied to the whole set, defined by artifact coupling and
placed by runtime capability:
- New `Client/tests/contract/`, holding
`server-admin-static-channel-perms.test.ts`. Same directory depth, so
`../../../Server/...` still resolves; the body is byte-identical apart from
a header naming the owner and the runner.
- `Server/updater/tauri_key_contract_test.go` splits the one cross-component
Go test out of `updater_test.go` verbatim, same `package updater`. It stays
in Go — placement follows capability, and Go parses JSON fine — so only the
file name has to declare the crossing. Without this the item would have
been "moved one file and declared the class closed".
- `npm run test:contract`, and the tier, the membership rule and a
blocking/non-blocking table in `docs/contributing.md#testing`, which
previously described no tiers at all.
- `Client/CLAUDE.md`'s tier list was missing `tests/e2e/admin` and
`tests/e2e/native` before this; it now lists all seven and states the rule.
`Server/CLAUDE.md` records why the SPA's execution-level invariant is
locked from the client tree, so nobody "fixes" it into a regex.
- Ledger `OC-0154.fix.test` re-pointed and `FINDINGS.md` re-rendered;
`.claude/workflows/bughunt.js` — the workflow that produced OC-0154 — no
longer describes the TS test surface as `tests/unit/*.test.ts` only.
- Three stale cross-stack pointers of exactly the class this item is about:
`tests/e2e/helpers.ts:348,351` and `tests/unit/types.test.ts:13` named
`docs/brain/06-Specs/PROTOCOL.md`, which does not exist (`docs/brain/` is a
gitignored path); all now name `docs/protocol.md`.
15 files, 125 insertions, 33 deletions. No CI job, workflow, vitest,
tsconfig, eslint, knip or stryker change, and no new pinned check —
`ci.yml`'s `npx vitest run --coverage` has no path filter and
`vitest.config.ts` includes `tests/**/*.test.ts`, so enforcement after the
move is bit-identical to enforcement before it. That is deliberate: `dev`
pins 11 contexts and a 12th is a branch-protection API write, not something a
PR can do, so any new job would be advisory until someone separately changed
repository settings — strictly less protection than today.
Verified: both directions, and the assertion was not weakened. Flipping
`admin/static/index.html:1182` to `const targetIsTouchedRole=false;` makes
the moved test fail (`AssertionError: expected 'DELETE' not to be 'DELETE'`);
`git checkout` of that file makes it pass again — so the invariant survived
the move intact rather than becoming a test that passes anywhere. The split
Go test's cross-boundary read is live too: with
`Client/src-tauri/tauri.conf.json` moved away, `go test ./updater/` fails
with `ReadFile(../../Client/src-tauri/tauri.conf.json): no such file or
directory` from `tauri_key_contract_test.go:20`, and passes once restored.
The full client suite is 192 files / 5257 tests passing, identical to the
count before the move; `npm run typecheck` passes, which proves
`tests/contract/` is inside the tsconfig graph and that `tests/types/jsdom.d.ts`
still resolves the moved test's `import { JSDOM }`. `npm run lint`,
`npx prettier --check .`, `go vet ./...` and `go test ./updater/` all pass.
`git grep "tests/unit/admin-static-channel-perms"` finds no survivor outside
the B1 plan itself.
Not included: nothing was deleted, because no e2e sibling covers OC-0154.
`Client/tests/types/jsdom.d.ts` was neither moved nor deleted — it is still
the only type source for the moved test's `jsdom` import. `capabilities-scope.test.ts`
and `tauri-conf-webview2-args.test.ts` read `src-tauri/` and stay in
`tests/unit`: `src-tauri` is inside the `Client` component, so they are not
contract tests, and the rule earns that rather than hand-waving it — moving
them would have forced repoints of ledger entry OC-0089 and
`docs/security.md:64` for no gain. Each gained a one-line header saying why.
`Server/admin/perm_grid_test.go` and `emoji_section_test.go` read their own
package's embedded asset and are unchanged; they are the text-level
complement to the execution-level test, not duplicates. No JS engine was
added to `go.mod`, no npm root was created under `Server/`, and no root-level
`tests/` tier was created — there is no runner for one and no way to make it
blocking from a PR. Separately noticed and NOT fixed here:
`docs/contributing.md:221` still says "All ten required checks" while
`docs/plans/b0-dev-branch-protection.sh` pins eleven since B1-3 added
`Repository Hygiene`, and `docs/plans/hp-0-scorecard-2026-08-25.md:109` is
stale the same way — that is the branch-protection item's to fix, not this
one's, and one register item per commit.
Refs RL-11, L-11
This commit is contained in:
Claude
@@ -120,7 +120,7 @@ Method:
|
||||
structural reference is evidence of coupling, not of a bug; open the cited file and confirm.
|
||||
2. For every candidate, grep for ALL callers before judging - a guard may already live upstream.
|
||||
3. Check whether an existing test already locks the behavior you think is wrong. If a test asserts it,
|
||||
it is intended behavior, not a bug. Test files are *_test.go and tests/unit/*.test.ts.
|
||||
it is intended behavior, not a bug. Test files are *_test.go, tests/unit/*.test.ts and tests/contract/*.test.ts.
|
||||
4. Report EVERY finding you can prove - there is no cap. The quality bar stays: zero findings is a
|
||||
valid, respectable answer, and each finding needs file, line, and a concrete repro.
|
||||
|
||||
@@ -200,7 +200,7 @@ const SURFACE_LENSES = [
|
||||
`an entity when events arrive out of order; read-state that can mark unread messages read, or lose an unread ` +
|
||||
`count, across a reconnect; an async handler whose await lets stale state be written after a newer update ` +
|
||||
`(last-write-wins race); a route guard bypassable by a rapid navigation sequence.\n` +
|
||||
`Check tests/unit/ before reporting - much of this behavior is already test-locked.`,
|
||||
`Check tests/unit/ and tests/contract/ before reporting - much of this behavior is already test-locked.`,
|
||||
},
|
||||
];
|
||||
|
||||
|
||||
Generated
+1
-1
@@ -4742,7 +4742,7 @@ saveChannelPerms writes the quick "Can access" toggles first (PUT allow=0/deny=0
|
||||
|
||||
**Suggested fix:** In saveChannelPerms, record the role IDs the quick-toggle loop actually wrote and skip the matrix step when the selected target is one of them — one guard in the one function: collect `const touched=new Set()` in the loop (add role.role_id on each PUT/DELETE), then wrap the matrix block in `if(path && !(permTargetPath().indexOf('/permissions/')>-1 && touched.has(tid)))`. Cleanest variant: give the quick checkbox an onchange that patches the in-memory role.allow/role.deny in state.permChannel and calls renderPermMatrix(), so the matrix always reflects the pending toggle instead of the stale snapshot.
|
||||
|
||||
**Fixed:** `69258a51` · test `Client/tests/unit/admin-static-channel-perms.test.ts` · revert-proof pass
|
||||
**Fixed:** `69258a51` · test `Client/tests/contract/server-admin-static-channel-perms.test.ts` · revert-proof pass
|
||||
|
||||
### OC-0155 — medium — Room-key offer pacing budget is per-call, so two back-to-back rotations blow the server's per-second offer cap and strand peers on a dead key
|
||||
|
||||
|
||||
@@ -3775,7 +3775,7 @@
|
||||
"confidence": "high",
|
||||
"fix": {
|
||||
"commit": "69258a51",
|
||||
"test": "Client/tests/unit/admin-static-channel-perms.test.ts",
|
||||
"test": "Client/tests/contract/server-admin-static-channel-perms.test.ts",
|
||||
"revertProof": "pass"
|
||||
},
|
||||
"suggestedFix": "In saveChannelPerms, record the role IDs the quick-toggle loop actually wrote and skip the matrix step when the selected target is one of them — one guard in the one function: collect `const touched=new Set()` in the loop (add role.role_id on each PUT/DELETE), then wrap the matrix block in `if(path && !(permTargetPath().indexOf('/permissions/')>-1 && touched.has(tid)))`. Cleanest variant: give the quick checkbox an onchange that patches the in-memory role.allow/role.deny in state.permChannel and calls renderPermMatrix(), so the matrix always reflects the pending toggle instead of the stale snapshot.",
|
||||
|
||||
+6
-1
@@ -9,8 +9,13 @@ Rust backend in `src-tauri/` for native APIs only. LiveKit handles voice/video.
|
||||
`src/pages/`, `src/components/` UI
|
||||
- `src/lib/protocolTypes.ts` and `src/generated/` are generated — see the root
|
||||
CLAUDE.md
|
||||
- `tests/unit`, `tests/integration` (vitest, jsdom) · `tests/e2e` (Playwright) ·
|
||||
- `tests/unit`, `tests/integration`, `tests/contract` (vitest, jsdom) ·
|
||||
`tests/e2e`, `tests/e2e/admin`, `tests/e2e/native` (Playwright) ·
|
||||
`tests/browser` (vitest browser mode)
|
||||
- A test whose assertions read, import or execute a **`Server/`-owned**
|
||||
artifact belongs in `tests/contract`, not `tests/unit` — `src-tauri/` is
|
||||
part of this component, so reading it is an ordinary unit test. The rule
|
||||
is in [docs/contributing.md](../docs/contributing.md#testing)
|
||||
|
||||
## Gotchas
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
"test": "vitest run",
|
||||
"test:unit": "vitest run tests/unit",
|
||||
"test:integration": "vitest run tests/integration",
|
||||
"test:contract": "vitest run tests/contract",
|
||||
"test:e2e": "playwright test",
|
||||
"test:e2e:prod": "npm run build && playwright test --config playwright.config.prod.ts",
|
||||
"test:e2e:native": "playwright test --config playwright.config.native.ts",
|
||||
|
||||
+8
@@ -1,3 +1,8 @@
|
||||
// CONTRACT TEST. The artifact under test is owned by Server/admin; the runner
|
||||
// lives here because placement follows capability, not ownership — the Go
|
||||
// module carries no JavaScript engine, so nothing under Server/ can execute
|
||||
// this SPA. See docs/contributing.md#testing for the membership rule.
|
||||
//
|
||||
// Loads the real Server/admin/static/index.html (the Go admin panel's
|
||||
// single-file SPA) into a scripted jsdom window and drives its inline
|
||||
// channel-permissions logic directly, the same way a browser would.
|
||||
@@ -5,6 +10,9 @@
|
||||
// There is no bundler or module system for this file — it is one inline
|
||||
// <script> executed as a classic script — so the only faithful way to test
|
||||
// it is to actually run it, not to re-implement its logic in TypeScript.
|
||||
// A text-level assertion is not a substitute: Server/admin/perm_grid_test.go
|
||||
// greps the same file, but flipping the guard at index.html:1182 to `false`
|
||||
// reintroduces the bug while leaving every greppable identifier intact.
|
||||
import { describe, it, expect, afterEach } from "vitest";
|
||||
import { JSDOM } from "jsdom";
|
||||
import { readFileSync } from "node:fs";
|
||||
@@ -345,10 +345,10 @@ export function chatEchoHandlers(): Array<{ type: string; handler: string }> {
|
||||
/**
|
||||
* Voice WS flow handlers for E2E testing.
|
||||
* Simulates the server-side voice protocol defined in:
|
||||
* docs/brain/06-Specs/PROTOCOL.md (voice_join, voice_leave, voice_token, voice_token_refresh)
|
||||
* docs/protocol.md (voice_join, voice_leave, voice_token, voice_token_refresh)
|
||||
* protocol/schema.json (message type schemas)
|
||||
*
|
||||
* When PROTOCOL.md voice message types change, update these handlers to match.
|
||||
* When docs/protocol.md voice message types change, update these handlers to match.
|
||||
*/
|
||||
export function voiceWsHandlers(): Array<{ type: string; handler: string }> {
|
||||
return [
|
||||
|
||||
Vendored
+4
-2
@@ -1,6 +1,8 @@
|
||||
// jsdom ships no type declarations and @types/jsdom is not a dependency of
|
||||
// this project. Declare the surface the admin-panel test actually uses,
|
||||
// following the same pattern as src/types/jitsi-rnnoise.d.ts.
|
||||
// this project. Declare the surface the tests/contract/ admin-SPA test
|
||||
// actually uses, following the same pattern as src/types/jitsi-rnnoise.d.ts.
|
||||
// (jsdom itself stays a required dependency regardless: vitest.config.ts
|
||||
// sets environment: "jsdom" for the whole suite.)
|
||||
declare module "jsdom" {
|
||||
export interface JSDOMOptions {
|
||||
url?: string;
|
||||
|
||||
@@ -10,6 +10,9 @@
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
// Asserts src-tauri/capabilities/default.json, which is inside the Client
|
||||
// component — not a cross-component contract test. See
|
||||
// docs/contributing.md#testing.
|
||||
import capabilityJson from "../../src-tauri/capabilities/default.json";
|
||||
|
||||
interface ScopeEntry {
|
||||
|
||||
@@ -12,6 +12,8 @@
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
// Asserts src-tauri/tauri.conf.json, which is inside the Client component —
|
||||
// not a cross-component contract test. See docs/contributing.md#testing.
|
||||
import tauriConf from "../../src-tauri/tauri.conf.json";
|
||||
|
||||
describe("tauri.conf.json — Windows WebView2 additionalBrowserArgs", () => {
|
||||
|
||||
@@ -10,7 +10,7 @@ import type {
|
||||
} from "../../src/lib/types";
|
||||
import { Permission as P } from "../../src/lib/types";
|
||||
|
||||
// Sample PROTOCOL.md JSON payloads for parsing validation
|
||||
// Sample docs/protocol.md JSON payloads for parsing validation
|
||||
const sampleAuthOk = {
|
||||
type: "auth_ok" as const,
|
||||
payload: {
|
||||
|
||||
@@ -22,6 +22,11 @@ prometheus.
|
||||
- Build tags gate whole files, so all four variants must compile: default,
|
||||
`-tags otel`, `-tags wazero`, `-tags otel,wazero`. Tests must also pass under
|
||||
`-race` and under `-tags deadlock`. The `ci-check` skill has the commands.
|
||||
- `admin/static/index.html` is server-owned, and its invariants are locked from
|
||||
two places: text-level ones from `admin/perm_grid_test.go` and
|
||||
`admin/emoji_section_test.go`, execution-level ones from
|
||||
`Client/tests/contract/` — Go has no JS engine, so a Go port could only grep.
|
||||
Do not "fix" that split by rewriting the contract test as a regex.
|
||||
- `ws` is the hub: broadcast fan-out, per-client send queues, replay, and voice
|
||||
state all interact under several locks. Sequenced frames share one per-client
|
||||
FIFO because clients ack only `max(seq)` — a frame that skips the queue, or a
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
package updater
|
||||
|
||||
// CONTRACT TEST. It reads an artifact owned by the Client component
|
||||
// (Client/src-tauri/tauri.conf.json) but asserts a Server constant, using the
|
||||
// client value only as a reference — so placement stays here, in the package
|
||||
// that owns the assertion, and only the file name has to declare the crossing.
|
||||
// See docs/contributing.md#testing for the membership rule.
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestDefaultServerSignaturePublicKey_DiffersFromTauriUpdaterKey(t *testing.T) {
|
||||
tauriConfigPath := filepath.Clean(filepath.Join("..", "..", "Client", "src-tauri", "tauri.conf.json"))
|
||||
raw, err := os.ReadFile(tauriConfigPath)
|
||||
if err != nil {
|
||||
t.Fatalf("ReadFile(%s): %v", tauriConfigPath, err)
|
||||
}
|
||||
|
||||
var cfg struct {
|
||||
Plugins struct {
|
||||
Updater struct {
|
||||
PubKey string `json:"pubkey"`
|
||||
} `json:"updater"`
|
||||
} `json:"plugins"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &cfg); err != nil {
|
||||
t.Fatalf("Unmarshal tauri.conf.json: %v", err)
|
||||
}
|
||||
|
||||
if cfg.Plugins.Updater.PubKey == defaultServerSignaturePublicKey {
|
||||
t.Fatalf("server updater signing key must differ from tauri.conf.json updater pubkey")
|
||||
}
|
||||
}
|
||||
@@ -627,29 +627,6 @@ func TestAssetFilenameFromURL(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultServerSignaturePublicKey_DiffersFromTauriUpdaterKey(t *testing.T) {
|
||||
tauriConfigPath := filepath.Clean(filepath.Join("..", "..", "Client", "src-tauri", "tauri.conf.json"))
|
||||
raw, err := os.ReadFile(tauriConfigPath)
|
||||
if err != nil {
|
||||
t.Fatalf("ReadFile(%s): %v", tauriConfigPath, err)
|
||||
}
|
||||
|
||||
var cfg struct {
|
||||
Plugins struct {
|
||||
Updater struct {
|
||||
PubKey string `json:"pubkey"`
|
||||
} `json:"updater"`
|
||||
} `json:"plugins"`
|
||||
}
|
||||
if err := json.Unmarshal(raw, &cfg); err != nil {
|
||||
t.Fatalf("Unmarshal tauri.conf.json: %v", err)
|
||||
}
|
||||
|
||||
if cfg.Plugins.Updater.PubKey == defaultServerSignaturePublicKey {
|
||||
t.Fatalf("server updater signing key must differ from tauri.conf.json updater pubkey")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDefaultServerSignaturePublicKey_Parseable(t *testing.T) {
|
||||
u := NewUpdater("1.0.0", "", "J3vb", "OwnCord")
|
||||
if _, err := u.serverSignaturePublicKey(); err != nil {
|
||||
|
||||
@@ -92,8 +92,10 @@ next section, and using them directly is equally correct.
|
||||
| `npm test` | Run all tests (vitest) |
|
||||
| `npm run test:unit` | Unit tests only |
|
||||
| `npm run test:integration` | Integration tests only |
|
||||
| `npm run test:contract` | Cross-component contract tests only |
|
||||
| `npm run test:e2e` | Playwright E2E (mocked Tauri) |
|
||||
| `npm run test:e2e:native` | Playwright E2E (real Tauri exe + CDP) |
|
||||
| `npm run test:e2e:admin` | Playwright E2E (real Go server + SPA) |
|
||||
| `npm run test:e2e:prod` | Playwright E2E (prod build) |
|
||||
| `npm run test:e2e:ui` | Playwright UI mode |
|
||||
| `npm run test:watch` | Vitest watch mode |
|
||||
@@ -230,6 +232,56 @@ the Go suite has deliberately no floor (T-2026-07-25-19) — use `make cover-all
|
||||
to see the honest cross-package number. Follow a test-driven workflow and never
|
||||
lower a threshold to make a change fit.
|
||||
|
||||
### Tiers
|
||||
|
||||
| Tier | Command | CI job | Blocking |
|
||||
| -------------------------- | -------------------------- | ------------------------------------------- | -------- |
|
||||
| `Client/tests/unit` | `npm run test:unit` | Client Unit Tests | yes |
|
||||
| `Client/tests/integration` | `npm run test:integration` | Client Unit Tests | yes |
|
||||
| `Client/tests/contract` | `npm run test:contract` | Client Unit Tests | yes |
|
||||
| `Client/tests/browser` | `npm run test:browser` | — | no |
|
||||
| `Client/tests/e2e` | `npm run test:e2e` | Client E2E (Playwright) | yes |
|
||||
| `Client/tests/e2e` @parity | — | Client E2E (parity subset, blocking) | yes |
|
||||
| `Client/tests/e2e/native` | `npm run test:e2e:native` | — | no |
|
||||
| `Client/tests/e2e/admin` | `npm run test:e2e:admin` | Admin Panel E2E (real server, non-blocking) | **no** |
|
||||
| `Server/**/*_test.go` | `make test` | Server Build & Test | yes |
|
||||
| `Client/src-tauri` | `cargo test --lib` | Rust Unit Tests | yes |
|
||||
|
||||
`npm test` — not `npm run test:unit` — is what CI runs and what
|
||||
`npm run check:client` invokes, so it is the command that covers
|
||||
`tests/contract`.
|
||||
|
||||
### What belongs in `tests/contract`
|
||||
|
||||
A test is a **contract test** when its assertions read, import or execute an
|
||||
artifact owned by a _different top-level component_ (`Server/`, `Client/`, root
|
||||
`protocol/`) than the one its runner lives in. A comment referencing the other
|
||||
side does not count.
|
||||
|
||||
1. **Placement follows capability, not ownership.** A contract test lives in the
|
||||
tier whose runtime can execute or parse the artifact. If the owning component
|
||||
can execute it, it stays in that component's own suite —
|
||||
`Server/updater/tauri_key_contract_test.go` reads
|
||||
`Client/src-tauri/tauri.conf.json` and stays in Go, because Go parses JSON
|
||||
fine and the assertion is about a server constant.
|
||||
2. **Ownership is declared in the name, never in the directory.** The file name
|
||||
and the top-level `describe`/`Test` name must name the owned artifact's path.
|
||||
3. **A contract test may only live in a blocking tier.** A non-blocking job is
|
||||
not coverage. `Admin Panel E2E` is `continue-on-error: true`
|
||||
(`.github/workflows/ci.yml`), so it is ineligible however well it fits
|
||||
topically — until it graduates.
|
||||
4. `Client/` is one component: its TypeScript frontend and its thin Rust backend
|
||||
in `src-tauri/` are the same side of the boundary, so a `tests/unit` test that
|
||||
reads `src-tauri/tauri.conf.json` is an ordinary unit test. The same goes for a
|
||||
Go test reading its own package's embedded assets
|
||||
(`Server/admin/perm_grid_test.go`).
|
||||
|
||||
E2E is _runtime_ coupling rather than artifact coupling; it stays in `tests/e2e`.
|
||||
|
||||
If a tier ever gains a runner of its own, model its anti-vacuity guard on
|
||||
`Server/invariants/invariants_test.go` — it fails loudly when a configured scope
|
||||
resolves to nothing, rather than passing on an empty set.
|
||||
|
||||
## Code Style
|
||||
|
||||
- **TypeScript**: See [Client Architecture](architecture/client.md)
|
||||
|
||||
Reference in New Issue
Block a user