Merge fix/8wy9-restore-rebuild (b32a7690) into fix/3til-restore-order
This commit is contained in:
Edward Langley
@@ -53,29 +53,6 @@ jobs:
|
||||
# below, so it fails under this job's name rather than another's.
|
||||
- name: The stamp gate refuses what it claims to
|
||||
run: ./scripts/checkers.sh assemble-stamp
|
||||
# scripts/web-toolchain-versions.env stays the one home for the pins, so
|
||||
# the release capture (scripts/build-web-release.sh, improvise-oujb)
|
||||
# provisions the same versions. CI no longer installs them — the dev
|
||||
# shell supplies wasm-pack and node — so this checks the shell against
|
||||
# the pins instead: a nixpkgs bump that moves either fails here, by name.
|
||||
# wasm-pack runs its own bundled `wasm-opt`, which is why
|
||||
# improvise-wasm-client's Cargo.toml names `--enable-bulk-memory`.
|
||||
- name: The dev shell supplies the pinned web toolchain
|
||||
run: |
|
||||
. scripts/web-toolchain-versions.env
|
||||
test "$(wasm-pack --version)" = "wasm-pack ${WASM_PACK_VERSION#v}"
|
||||
test "$(node -p 'process.versions.node.split(".")[0]')" = "$NODE_VERSION"
|
||||
wasm-pack --version
|
||||
node --version
|
||||
# wasm-pack uses the shell's wasm-bindgen only at Cargo.lock's exact
|
||||
# version; otherwise it cargo-installs one (71s) — so a lock bump
|
||||
# must move flake.nix's wasmBindgenCli too, and this says so.
|
||||
lock="$(awk '/^name = "wasm-bindgen"$/ {getline; gsub(/[^0-9.]/, ""); print}' Cargo.lock)"
|
||||
test "$(wasm-bindgen --version)" = "wasm-bindgen $lock" || {
|
||||
echo "flake.nix wasmBindgenCli is $(wasm-bindgen --version); Cargo.lock wants $lock" >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
# Built by Nix (packages.web and packages.site in nix/ci.nix), which runs
|
||||
# the same harness/build.sh and web/assemble.sh a developer runs, so there
|
||||
# is still one answer to "how is the bundle built". `web` is the three
|
||||
@@ -84,14 +61,12 @@ jobs:
|
||||
# the harness tree is stamped `harness`. Dependencies are their own
|
||||
# derivations, substituted from titan's cache when Cargo.lock is unchanged.
|
||||
- name: Build the bundles, the harness tree and the site
|
||||
shell: bash
|
||||
run: |
|
||||
nix build --print-build-logs --out-link result-web .#web
|
||||
nix build --print-build-logs --out-link result-site .#site
|
||||
# The upload steps below read these paths. Copied rather than linked:
|
||||
# an artifact of a symlink into /nix/store is a dangling link elsewhere.
|
||||
- name: Lay the outputs out where the uploads read them
|
||||
shell: bash
|
||||
run: |
|
||||
rm -rf harness/serve web/pkg web/worker-pkg site
|
||||
cp -rL result-web/harness-serve harness/serve
|
||||
@@ -143,12 +118,10 @@ jobs:
|
||||
# checks.clippy and checks.test in nix/ci.nix: the same cargo commands,
|
||||
# on the dev/test profiles, with dependencies as a cached derivation.
|
||||
- name: cargo clippy --workspace --all-targets -- -D warnings
|
||||
shell: bash
|
||||
run: nix build --print-build-logs .#checks.x86_64-linux.clippy
|
||||
# Also runs the test-evidence gate (check-comment-runs.py
|
||||
# --check-evidence), which lists the tests this derivation compiled.
|
||||
- name: cargo test --workspace, and test-evidence headings name real tests
|
||||
shell: bash
|
||||
run: nix build --print-build-logs .#checks.x86_64-linux.test
|
||||
# The comment cap and the test-evidence keys (improvise-9c8c); without
|
||||
# this step the rule is advisory. See context/design-principles.md §6.
|
||||
@@ -174,9 +147,7 @@ jobs:
|
||||
runs-on: [self-hosted, nixos, microvm]
|
||||
steps:
|
||||
- uses: actions/checkout@v7
|
||||
# Plain bash, not the dev shell: this step needs nix and nothing else.
|
||||
- name: nix build .#improvise
|
||||
shell: bash
|
||||
run: nix build --print-build-logs .#improvise
|
||||
|
||||
wasm:
|
||||
@@ -190,7 +161,6 @@ jobs:
|
||||
# depends on crossterm, which does not build for wasm — that is correct,
|
||||
# not a gap (improvise-ywd). The log names each crate as it is checked.
|
||||
- name: cargo check --target wasm32-unknown-unknown, per bundle crate
|
||||
shell: bash
|
||||
run: nix build --print-build-logs .#checks.x86_64-linux.wasm-check
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -235,9 +205,9 @@ jobs:
|
||||
# bypass also found that nothing ran this suite.
|
||||
- name: The kb tool refuses what it claims to
|
||||
run: ./scripts/checkers.sh kb
|
||||
# Every suite goes through scripts/checkers.sh, which pins a bash >= 5
|
||||
# and scrubs the environment, so the shell and the variables a suite
|
||||
# passes under here are the ones it passes under locally (improvise-f9qf).
|
||||
# Every suite goes through scripts/checkers.sh, which scrubs the
|
||||
# environment, so the variables a suite passes under here are the ones it
|
||||
# passes under locally (improvise-f9qf). Its bash is the dev shell's.
|
||||
- name: The checker runner refuses what it claims to
|
||||
run: ./scripts/checkers.sh --self-test
|
||||
# pr-watch is driven entirely from fixtures. The release preflight is the
|
||||
@@ -273,7 +243,6 @@ jobs:
|
||||
# checks.harness-lint in nix/ci.nix, run from inside harness/opfs-contract:
|
||||
# `cargo fmt -p opfs-contract -- --check` and clippy for wasm32.
|
||||
- name: cargo fmt -p opfs-contract and clippy --target wasm32
|
||||
shell: bash
|
||||
run: nix build --print-build-logs .#checks.x86_64-linux.harness-lint
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -301,19 +270,9 @@ jobs:
|
||||
- name: Install the harness
|
||||
working-directory: harness
|
||||
run: npm ci
|
||||
# No browser install step: the dev shell points PLAYWRIGHT_BROWSERS_PATH
|
||||
# at nixpkgs' Chromium, Firefox and WebKit, since Playwright's own
|
||||
# downloads cannot run on NixOS. All three, because improvise-xf1's
|
||||
# acceptance criterion names Chrome, Firefox and Safari and
|
||||
# `createSyncAccessHandle` is implemented separately by each engine.
|
||||
# WebKit's OPFS is absent today; the spec skips on a runtime probe.
|
||||
# @playwright/test must equal nixpkgs' playwright-driver version, or it
|
||||
# looks for browser builds that are not there; this says which.
|
||||
- name: Playwright matches the dev shell's browsers
|
||||
working-directory: harness
|
||||
run: |
|
||||
npx playwright --version
|
||||
ls "$PLAYWRIGHT_BROWSERS_PATH"
|
||||
# No browser install step and no version probe: the dev shell sets
|
||||
# PLAYWRIGHT_BROWSERS_PATH to nixpkgs' browsers, which pin them. `npm ci`
|
||||
# stays because @playwright/test itself is not nix-provided.
|
||||
# The tree `build` assembled, not one this job rebuilds. Same bytes as
|
||||
# the Pages artifact, modulo the cache-bust token `web/assemble.sh`
|
||||
# stamps — the harness tree is stamped `harness` and the site the commit
|
||||
|
||||
+101
-1
@@ -1997,7 +1997,107 @@ edit are the same string **by construction** rather than by agreement
|
||||
(improvise-k8h) — the keymap never spells a buffer name.
|
||||
|
||||
`commit(key, cmd, args)` welds the command to its `clear-buffer` so no caller
|
||||
can bind a commit without its clear (improvise-w3q). `rebind` exists for
|
||||
can bind a commit without its clear (improvise-w3q).
|
||||
|
||||
**The two cell-editing commit keys are the exception, and the clear moved rather
|
||||
than disappeared** (improvise-fy9o). `commit-cell-edit` and
|
||||
`commit-and-advance-right` reopen an editor on the cell they advance to, and the
|
||||
welded clear ran **after** them, so it blanked the re-seeded buffer. The
|
||||
reopened editor looked empty, and a second Enter committed `""` and cleared the
|
||||
landed cell. The user's ruling is that the reopened editor shows the landed
|
||||
cell's current value, so with improvise-ixxx's untouched rule an unchanged Enter
|
||||
writes nothing. `commit_reopening(key, cmd, args)` therefore binds the command
|
||||
alone, and the command clears its own buffer **at its head**:
|
||||
- in the TUI, `CommitAndAdvance` does it right after the mode change;
|
||||
- in the browser, `resolve::cleared` does it, reached from both `commit` and
|
||||
`create_column`.
|
||||
|
||||
Being at the head is also gotcha 21's rule: a head clear survives an abort
|
||||
where the trailing weld did not. The corner commit and the `+cat` insertion
|
||||
reopen nothing, and they still leave no text behind.
|
||||
|
||||
**After a write, the browser's reopened editor takes its seed from the host's
|
||||
answer, not from its cache** (improvise-3uzg). The first fix (improvise-3tt4)
|
||||
seeded from the cache everywhere except the cell the frame had just written.
|
||||
That assumed the write changed the meaning of only that one cell, which is
|
||||
false. Under `prune_empty`, emptying the last value in a row takes the row
|
||||
away. The advance then lands on a renumbered position, and the cache still
|
||||
holds the old record there. The browser opened on South's `2` while the TUI
|
||||
opened on West's `3`, and typing `5` wrote 25 into West. Any write can change
|
||||
what another position means, and the client cannot tell which. So the rule is
|
||||
stated once, over the write, never over a list of cases:
|
||||
- **A commit that writes** reopens its editor as `OpenedEditor::awaiting()`, an
|
||||
empty buffer whose seed is still to come. `create_column` is included,
|
||||
because `AddItem` adds a column.
|
||||
- **`ClientState::settle_awaited_seed`** fills the seed from the grid the
|
||||
client holds, but only once no frame it sent is still unanswered. The answer
|
||||
folds the host's projection before `Accepted`, so the grid is the host's by
|
||||
then. `accepted()` therefore settles.
|
||||
- **A commit that writes nothing** awaits as well while any earlier frame's
|
||||
write is unanswered. Otherwise it seeds from the cache at once. "Nothing
|
||||
changed, so the cache is current" was the rule here, and it was false
|
||||
(improvise-mdfc). *This* commit changed nothing, but an earlier one might
|
||||
have, and the cache predates it until the answer arrives. Tab Tab pressed
|
||||
quickly at the rightmost column opened the second editor on the value from
|
||||
before the first Tab's write: 505 where the TUI stores 5095. So the test is
|
||||
"does this frame write, or is a write still in flight", which is
|
||||
`writes || in_flight.iter().any(|sent| sent.awaited)`. It checks *any*
|
||||
in-flight write, not the newest frame: a frame that writes nothing stays in
|
||||
the queue behind an unanswered write, so the newest frame can be one of
|
||||
those.
|
||||
- **Every opener asks the in-flight half, because it lives in `opened_on`**
|
||||
(improvise-p6v6). mdfc first put the whole test inside `advanced`, which left
|
||||
`i`/`a` (`edit_or_drill`) seeding from the cache with no in-flight check.
|
||||
`i 9 Tab Esc i 5 Enter`, typed faster than the answer, stored 15 in the
|
||||
browser where the TUI stores 195. The two halves now live in different
|
||||
places, for a reason. "A write is still in flight" is true of the *client*,
|
||||
so `opened_on` asks it, and no opener can forget. "This frame writes" is
|
||||
true only of the commit being resolved, which is not in `in_flight` yet, so
|
||||
`advanced` still asks it itself. Folding both into `opened_on` would lose the
|
||||
second: the 3tt4 and 3uzg tests fail when `advanced` defers entirely.
|
||||
The click-edit path (`socket.rs::edit_target_at`, improvise-ugj4) also seeds
|
||||
from the cache, but it claims the record at the version it *read*. Behind an
|
||||
unanswered write the host therefore refuses its commit as `RecordMoved`: a
|
||||
loud failure, not a silent overwrite.
|
||||
- **The seed settles only into an open editor** (improvise-4cb5). An Esc typed
|
||||
before the answer arrives leaves the awaiting `opened_editor` in place. The
|
||||
late seed would then fill the edit buffer in `Normal` mode, where the TUI's is
|
||||
empty.
|
||||
- **`awaiting` has no `serde(default)`** (improvise-mgis), for the reason
|
||||
`AxisFrame::first` has none (§3a, improvise-1wl0). `awaiting: false` is a
|
||||
claim, "the seed is settled". A peer that dropped the field would hand over an
|
||||
editor that stays empty forever, and whose Enter writes nothing. A missing
|
||||
field is therefore a parse failure.
|
||||
|
||||
Asking the host to send the seed was the alternative. It would have needed a
|
||||
new message, and the host already sends it: the landed cell's `edit_text` in
|
||||
the projection it owes this client anyway. Two things are accepted costs:
|
||||
- Until the answer arrives, the editor is empty. Over a worker that takes
|
||||
well under a frame.
|
||||
- A key typed before the answer arrives marks the editor touched, and the late
|
||||
seed then does not overwrite it. What the user sees is then what gets
|
||||
written. That is never another record's number only because of the mdfc
|
||||
rule above: before it, an untouched commit behind an unanswered write opened
|
||||
on the stale cache, and typing then extended the wrong number. What is left
|
||||
is a divergence from the TUI rather than a wrong record. A key that beats the
|
||||
answer replaces the value the TUI would have extended: back to back,
|
||||
`i 9 Tab Tab 5 Enter` stores 5 or 5095 in Chromium over the worker
|
||||
(improvise-n4i9).
|
||||
|
||||
The TUI is unchanged: its `RebuildLayout` in front of `EnterEditAtCursor`
|
||||
already reads the renumbered grid (gotcha 23). This also took
|
||||
`CellValue::edit_text` out of the client bundle. That call formats an `f64`,
|
||||
which is what compiled to `i64.trunc_sat_f64_s`, so `a093d126`'s
|
||||
`nontrapping-float-to-int` flag on the wasm-client is reverted. A `?ws=` page
|
||||
loads only the client bundle, so it works without the feature.
|
||||
|
||||
Reordering the weld to `[clear-buffer, cmd]` was tried first and rejected. Four
|
||||
readers take a `Sequence`'s **first** step as its identity: `keymap::runs`,
|
||||
`help.rs`, and two resolver audits. So every hint on those keys started
|
||||
reporting `clear-buffer`. `text_entry_modes_clear_the_buffer_they_type_into`
|
||||
keeps its guarantee by asking a plain-`Cmd` commit key's command what it emits.
|
||||
|
||||
`rebind` exists for
|
||||
exactly one case: CommandMode's Backspace, which exits the mode on an empty
|
||||
`:` line instead of popping a character that isn't there.
|
||||
|
||||
|
||||
+30
-15
@@ -660,32 +660,47 @@ sixteen scripts, and the review cost is the whole diff rather than the shebang
|
||||
line. The runner also does not ship zsh, so every job would need an `apt-get`.
|
||||
Declined.
|
||||
|
||||
What was adopted instead pins the *interpreter and the environment together*, in
|
||||
What was adopted instead fixes the *environment* a suite runs in, in
|
||||
one place, with no per-invocation nix cost:
|
||||
|
||||
**`scripts/checkers.sh` is the one entry point for every check_the_checker.** CI
|
||||
calls it and the local gate calls it, so there is no second invocation to drift.
|
||||
It does three things a bare `./scripts/X-test.sh` does not:
|
||||
It runs every suite under the bash that runs it: in CI and under direnv, that
|
||||
is the dev shell's bash. It does two things a bare `./scripts/X-test.sh` does
|
||||
not:
|
||||
|
||||
1. **Pins a bash ≥ 5.1.** The ambient one is tried first, so a runner that
|
||||
already has bash 5.2 pays nothing; nix supplies one only when the ambient
|
||||
bash is older, which on macOS outside the dev shell it is. No bash ≥ 5.1 and
|
||||
no nix is **exit 3** — a refusal, never a quiet run under 3.2. The floor is
|
||||
5.1 and not 5 because `pr-reviews-test.sh` uses `wait -n -p`, and `-p`
|
||||
arrived in 5.1. Under 5.0 it is an invalid option (improvise-fd0j).
|
||||
2. **Scrubs the environment to an allowlist** (`env -i` plus `PATH`, `HOME`,
|
||||
1. **Scrubs the environment to an allowlist** (`env -i` plus `PATH`, `HOME`,
|
||||
`TMPDIR`, the `NIX_*` and `GITHUB_*` names, extensible through
|
||||
`CHECKERS_KEEP`). This is the part that closes the direnv hole above, and it
|
||||
is a *positive* list on purpose: the failure mode of a missing name is a loud
|
||||
"command not found", where the failure mode of an extra name is the silence
|
||||
this section is about.
|
||||
3. **Enforces the shebang rule**, so the `sh`/`bash` split cannot come back.
|
||||
2. **Enforces the shebang rule**, so the `sh`/`bash` split cannot come back.
|
||||
|
||||
The pinned bash's directory goes on the front of `PATH`, so a script the suite
|
||||
invokes through its *own* `#!/usr/bin/env bash` — or through `sh`, which
|
||||
nixpkgs' bash also provides — gets the pinned one too. Pinning only the outer
|
||||
shell would have left every inner one on the system bash, which is where the
|
||||
fault actually lived.
|
||||
**The runner no longer polices the bash version or where tools live.** It used
|
||||
to pin a bash of at least 5.1 (`find_modern_bash`: try the ambient bash, fall
|
||||
back to `nix shell nixpkgs#bash`, and exit 3 if neither was available). The
|
||||
floor was 5.1 because `pr-reviews-test.sh` uses `wait -n -p` (improvise-fd0j).
|
||||
All of that was removed on the user's ruling, when CI moved to the NixOS
|
||||
runners: "we shouldn't care which version of bash and other standard utilities
|
||||
are running in CI", and "most jobs should just take what comes in the ci dev
|
||||
shell".
|
||||
|
||||
So every CI step runs inside `nix develop .#ci`, which provides the bash and the
|
||||
GNU coreutils. The runner calls suites with `$BASH` and an unchanged `PATH`. A
|
||||
script that needs a newer bash feature fails where it uses it, not at a version
|
||||
gate.
|
||||
|
||||
Two things went with the floor:
|
||||
|
||||
- **Its self-tests.** `checkers-test.sh` sections 2, 5 and 5b faked old
|
||||
bashes, or stripped `PATH` down to hand-picked utilities, to prove the floor
|
||||
held. Those were the tests that broke on the NixOS runners, where `/usr/bin`
|
||||
and `/bin` hold only `env` and `sh` (#296, improvise-3uow).
|
||||
- **The hard-coded tool paths** in the suites (`/bin/sleep` and `/bin/date` in
|
||||
`pr-reviews-test.sh`) and `scripts/kb`'s BSD `stat -f` fallback. `scripts/kb`
|
||||
already needs the dev shell for `tpl`, so GNU `stat` is always present when it
|
||||
runs.
|
||||
|
||||
**A suite is `X-test.sh` where `X` or `X.sh` is present beside it.** That rule
|
||||
exists because `scripts/node-test.sh` matches `*-test.sh` and is a test
|
||||
|
||||
@@ -312,11 +312,14 @@ with an empty vec, so that would be a keypress the user sees do nothing at all
|
||||
|
||||
### enter_commits_the_cell_advances_and_re_enters_editing
|
||||
|
||||
Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]`, and every
|
||||
part of that sequence shows up in what the client composes — **including the
|
||||
trailing clear that empties the buffer `EnterEditAtCursor` has just seeded**.
|
||||
That ordering looks wrong and is exactly what the TUI does; mirroring it line
|
||||
for line is the contract.
|
||||
Enter in `Editing` used to be `[commit-cell-edit editing, clear-buffer edit]`,
|
||||
and the trailing clear emptied the buffer the advance had just re-seeded. That
|
||||
was improvise-fy9o. Since the fix, Enter is `commit-cell-edit` alone, and the
|
||||
commit clears its own buffer at its head. The test pins that order: the write,
|
||||
then `ChangeMode(Normal)`, then `SetBuffer(edit, "")`, then `SetSelected`. It
|
||||
ends with the re-entry, and the **last** write to `edit` is the landed cell's
|
||||
`"900"`. **Rival:** `resolve::commit` without its head clear fails this test,
|
||||
and `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends` too.
|
||||
|
||||
---
|
||||
|
||||
@@ -5172,6 +5175,75 @@ is touched. Enter must then land on (2, 0), and the folded `opened_editor` must
|
||||
be exactly `OpenedEditor::on("800")`. **Injection:** `resolve::advanced`
|
||||
carrying the previous editor's `touched` flag. This test fails alone.
|
||||
|
||||
### `enter_enter_leaves_every_cell_unchanged_on_both_ends`
|
||||
|
||||
**improvise-fy9o**, browser and host in lockstep. On the 2×2 `precise_host`,
|
||||
starting at (South, Cost), row 1 and column 0, the test presses `i`, Enter,
|
||||
Enter. The client resolves each key, folds it and applies it to one host, and
|
||||
`reduce_key` drives a second host. After each key the client's buffers must
|
||||
equal the key host's, and at the end every cell must be unchanged on both
|
||||
hosts.
|
||||
|
||||
Failed before the fix: (North, Revenue) was cleared, because the welded clear
|
||||
had blanked the reopened editor. **Rival:** the editing commit keys bound
|
||||
through the welding `.commit` again fails this test, the typing test below, and
|
||||
both TUI tests.
|
||||
|
||||
### `typing_into_the_reopened_editor_extends_the_cell_on_both_ends`
|
||||
|
||||
The other half of the ruling: the reopened editor shows the landed cell, so
|
||||
typing extends it. `i`, Enter, `5`, Enter must store `25` at (North, Revenue)
|
||||
on both ends. It fails under the weld rival, because the digit went into a
|
||||
blank buffer and stored `5`.
|
||||
|
||||
### `a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends`
|
||||
|
||||
Moving the clear from the keymap into the command must not leave text behind
|
||||
where nothing reopens. At the bottom-right corner, `i`, `9`, Enter must leave
|
||||
`edit` as `""` on both ends, with the mode no longer editing. **Rival:** the
|
||||
browser without `cleared` leaves `"49"` and fails here. The equivalent TUI rival
|
||||
fails `text_entry_modes_clear_the_buffer_they_type_into`.
|
||||
|
||||
### `tab_at_the_rightmost_column_reopens_on_the_value_just_written`
|
||||
|
||||
**improvise-3tt4**, found by the #227 review at `a1b23c05`. `resolve::advanced`
|
||||
re-seeded the reopened editor from the client's cache, on the argument that the
|
||||
advance always lands on a *different* cell. That is false for Tab at the
|
||||
rightmost column: `Advance::Right` clamps back onto the cell the frame has just
|
||||
written, and that cell's cache entry is the pre-write value.
|
||||
|
||||
The test uses the lockstep 2×2 fixture and starts at (0, 1), which is North /
|
||||
Revenue and holds 2. It presses `i 9 Tab 5 Enter` on a browser client and on a
|
||||
key-driven host, and compares the buffers after every key. It failed at
|
||||
`a1b23c05` on `Tab`: the browser buffer was `"2"` and the host's was `"29"`.
|
||||
Pressing `5 Enter` would then have written 25 over the 29 the user had just
|
||||
committed. Both ends must end at 295.
|
||||
|
||||
**Fix, superseded by improvise-3uzg:** the first fix, `landed_seed`, seeded
|
||||
from the committed text on the cursor's own cell. It assumed that cell was the
|
||||
only stale one, which prune disproved. After a write the editor now awaits the
|
||||
host's answer, so this test drives each key through `sent_and_answered`: the
|
||||
key is folded as an outgoing frame, the host applies it and projects, then
|
||||
`Accepted` arrives. Both rivals below still fail it.
|
||||
|
||||
### `tab_at_the_rightmost_column_reopens_on_the_stored_spelling`
|
||||
|
||||
This pins what "the committed value" means: the stored value's edit text, not
|
||||
the raw typed string. `i Backspace 0 0 7 Tab` at (0, 1) must leave `"7"` in both
|
||||
buffers. **Rival** (under the 3tt4 fix): seeding from the raw text failed with
|
||||
`"007"` against `"7"`. Under the 3uzg fix the seed comes from the host's
|
||||
projection, which has only the stored value to give. Opening the editor at once
|
||||
from the cache fails this test.
|
||||
|
||||
### `a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends`
|
||||
|
||||
**improvise-5wai**, found by the #227 review. `+cat` reopens nothing, so the
|
||||
commit's own head clear (`resolve::cleared` in `create_column`) is the only
|
||||
thing that empties the buffer. The test types `Season` at the `+cat` column of
|
||||
the records pair, presses Enter on both ends, and requires the category to have
|
||||
landed and both buffers to be `""`. **Rival:** `let _ = cleared(&edit_mode)` in
|
||||
`create_column` fails only this test, with the browser keeping `"Season"`.
|
||||
|
||||
### an_edit_opened_before_a_document_swap_is_refused_in_the_new_document
|
||||
|
||||
improvise-vndi, through a real `Host` over `MemoryFs`. A opens an inline editor
|
||||
@@ -5191,6 +5263,121 @@ the current one. That is the pre-fix behaviour, restated after the fix. Only
|
||||
this test failed in the whole workspace. The layout tier staying current is
|
||||
still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumber`.
|
||||
|
||||
### `a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up`
|
||||
|
||||
**improvise-3uzg**, found by the #227 review at `a093d126`. Fixture:
|
||||
North/South/West = 1/2/3 on one measure, `prune_empty` on, window 3×3. Keys
|
||||
`i Backspace Enter 5 Enter`, each through `sent_and_answered` on the browser and
|
||||
through `reduce_key` on the TUI host. The buffers must agree after every key,
|
||||
and the two documents must be identical at the end. The Backspace-Enter empties
|
||||
North, which prunes its row, and the advance lands on row 1, which is now West.
|
||||
|
||||
Red at `a093d126`: after the first Enter the browser buffer was `"2"` (South, the
|
||||
stale cache) and the host's was `"3"`. At gh/main both reopened blank.
|
||||
**Rivals** (each fails this test and the two `tab_at_the_rightmost_column_*`
|
||||
tests):
|
||||
- `settle_awaited_seed` not waiting for unanswered frames, so it seeds while
|
||||
folding its own outgoing frame.
|
||||
- `accepted()` not settling, so the seed never arrives.
|
||||
- The resolver seeding from the cache immediately after a write.
|
||||
|
||||
### `client::tests::an_awaited_seed_waits_for_the_answer_and_never_overwrites_typing`
|
||||
|
||||
The fold half of improvise-3uzg, without a host. The steps:
|
||||
1. A write frame with an awaiting editor is sent over a cache that says `2`.
|
||||
2. The host's projection saying `3` arrives. The buffer must still be empty.
|
||||
3. `Accepted` arrives. The buffer and the opened editor must be seeded `3`.
|
||||
4. In a second client, a keystroke `5` arrives before the answer. After
|
||||
`Accepted` the buffer must still be `5`.
|
||||
|
||||
**Rivals:**
|
||||
- Seeding while a frame is unanswered fails this test and the three lockstep
|
||||
tests.
|
||||
- `accepted()` not settling fails the same four.
|
||||
- Ignoring `touched` fails this test alone. It is the one guard on type-ahead.
|
||||
|
||||
### `a_tab_sent_before_the_last_write_is_answered_reopens_on_that_write`
|
||||
|
||||
**improvise-mdfc**, found by the #227 review at `baabe1a9`. Fixture:
|
||||
`precise_host`, cursor (0,1) on North/Revenue = 2 (the rightmost column), window
|
||||
2×2. `i 9` go through `sent_and_answered`. Then **three** Tabs are sent through
|
||||
`sent`, and none is answered until all three are out. The first writes 29 and
|
||||
the other two write nothing. After the answers are delivered in order, the
|
||||
browser's buffers must equal the TUI host's. `5 Enter` then has to store 295 on
|
||||
both ends.
|
||||
|
||||
Every earlier lockstep test answered each key before the next was sent, so the
|
||||
client's cache was always the host's by the time it resolved a key. That is why
|
||||
none of them saw this. The hazard needs a key resolved while a write is still
|
||||
unanswered.
|
||||
|
||||
Red at `baabe1a9`: after the answers the browser buffer was `"2"` (the cache from
|
||||
before the write) and the host's was `"29"`. In Chromium over the worker, with
|
||||
`tall.improv` R01/Cost = 50 and `i 9 Tab Tab 5 Enter`, the browser stored 505
|
||||
where the TUI stores 5095.
|
||||
|
||||
**Rivals:**
|
||||
- The fix reverted (await only when this commit writes) fails this test alone.
|
||||
The workspace was otherwise green at `baabe1a9`.
|
||||
- Awaiting only when the **newest** in-flight frame is awaited
|
||||
(`in_flight.back()`) fails this test, and the other 53 lockstep tests pass. That is why it sends three Tabs,
|
||||
not two: with two, the newest in-flight frame *is* the write, and `back()` and
|
||||
`any()` cannot be told apart.
|
||||
|
||||
### `client::tests::an_awaited_seed_arriving_after_escape_leaves_the_buffer_empty`
|
||||
|
||||
**improvise-4cb5**. A write frame with an awaiting editor is sent, and then Esc
|
||||
(`SetBuffer edit ""`, `ChangeMode Normal`) is sent before the answer arrives.
|
||||
The host's projection says `3`, and then `Accepted` arrives. The mode must be
|
||||
`Normal` and the edit buffer must be empty.
|
||||
|
||||
Red before the fix: the buffer was `"3"`. `settle_awaited_seed` asked only
|
||||
whether the opened editor was awaiting and untouched, and Esc leaves
|
||||
`opened_editor` in place. The seed `3` is distinct from the empty buffer Esc
|
||||
leaves, so the assertion can only pass if nothing was written.
|
||||
|
||||
**Rival:** the mode check removed (the code at `baabe1a9`) fails this test, and
|
||||
the rest of `improvise-protocol`'s lib passes.
|
||||
|
||||
### `resolve::opened_editor_wire_tests::a_frame_whose_editor_lost_its_awaiting_flag_does_not_parse`
|
||||
|
||||
**improvise-mgis**. A `ClientFrame::Commands` carrying
|
||||
`SetOpenedEditor(Some(OpenedEditor::awaiting()))` is serialized, and the
|
||||
`awaiting` key is removed from the JSON. The test asserts the key was there, and
|
||||
that `seed` is still there, so the mutated value is a well-formed editor minus
|
||||
one field. It must then fail to parse **as the missing field**, not as broken
|
||||
JSON.
|
||||
|
||||
**Rival:** `#[serde(default)]` restored. The frame then parses, with
|
||||
`awaiting: false`, and the test fails at `expect_err`. Before this test existed,
|
||||
deleting the attribute left the whole workspace green, so nothing pinned it.
|
||||
|
||||
### `an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write`
|
||||
|
||||
**improvise-p6v6**, found by the #227 review at `de4a7d78`. It predates #227:
|
||||
the reviewer reproduced it against gh/main `202e7eac`. Fixture: `precise_host`,
|
||||
cursor (0,1) on North/Revenue = 2 (the rightmost column), window 2×2.
|
||||
|
||||
1. `i 9` are answered one at a time.
|
||||
2. `Tab` (writes 29), `Escape` and `i` go out through `sent`, and none is
|
||||
answered until all three are out.
|
||||
3. The browser's buffers must then equal the TUI host's.
|
||||
4. `5 Enter` must store 295 on both ends.
|
||||
|
||||
Red at `de4a7d78`: the browser buffer was `"2"` where the TUI's was `"29"`. `i`
|
||||
reaches `edit_or_drill`, which called `opened_on(cell.edit_text)` with no
|
||||
in-flight check. mdfc had put that check inside `advanced` alone.
|
||||
|
||||
**Rivals:**
|
||||
- **The fix reverted** (the code at `de4a7d78`): fails this test only.
|
||||
- **`advanced` defers wholly to `opened_on`** (its own `writes` half dropped,
|
||||
on the reasoning that `opened_on` now checks): fails
|
||||
`a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up`, both
|
||||
`tab_at_the_rightmost_column_*` tests, and
|
||||
`resolve::tests::enter_commits_the_cell_advances_and_re_enters_editing`. The
|
||||
commit being resolved is not in `in_flight` yet, so `opened_on` cannot see its
|
||||
write.
|
||||
|
||||
### tab_in_an_editor_commits_and_lands_one_column_right
|
||||
|
||||
improvise-rq3f, found in the adversarial review of PR #246. Changing `advanced`'s
|
||||
|
||||
@@ -2891,6 +2891,11 @@ The buffer is derived from each mode rather than restated in the test —
|
||||
`every_text_entry_mode_types_into_the_buffer_its_prompt_names` owns pinning what it
|
||||
should be (improvise-k8h).
|
||||
|
||||
**Amended by improvise-fy9o.** A commit key bound to a plain command now passes only if that command, built in
|
||||
the mode with text in the buffer, emits `SetBuffer { name: <buffer>, value: "" }`
|
||||
itself. **Rival:** `CommitAndAdvance` without its head clear. Only this test
|
||||
fails, which makes it the TUI's guard against leftover text.
|
||||
|
||||
### minibuffer_enter_dispatches_to_a_registered_commit
|
||||
|
||||
improvise-6t3: the three commit commands collapsed into a single
|
||||
@@ -6235,8 +6240,9 @@ The buffer after `Enter` is **not** asserted. The keymap welds `clear-buffer`
|
||||
after every commit key, so it empties the buffer `EnterEditAtCursor` has just
|
||||
seeded, and the reopened editor opens blank while its captured seed is `"2"`.
|
||||
That is existing behaviour (`enter_commits_the_cell_advances_and_re_enters_editing`
|
||||
in `clients.md`). An `Enter` there with nothing typed therefore clears the landed cell,
|
||||
measured here; that is filed as improvise-fy9o rather than fixed here.
|
||||
in `clients.md`). An `Enter` there with nothing typed therefore cleared the landed
|
||||
cell. It was filed as improvise-fy9o and is now fixed (see
|
||||
`enter_enter_leaves_every_cell_unchanged`).
|
||||
|
||||
### `an_editor_typed_back_to_its_seed_still_writes`
|
||||
|
||||
@@ -6293,6 +6299,27 @@ flag. This test fails alone. Every other test stays green, which was the gap.
|
||||
Once improvise-fy9o re-seeds the reopened editor, a stale `touched` would make
|
||||
every untouched Tab or Enter write its seed.
|
||||
|
||||
### `enter_enter_leaves_every_cell_unchanged`
|
||||
|
||||
**improvise-fy9o.** The keymap welded `clear-buffer` after every commit key, so
|
||||
it blanked the editor `CommitAndAdvance` had just reopened. The next Enter then
|
||||
committed `""` and cleared the landed cell. The test presses `i`, Enter, Enter
|
||||
on the 2×2 fixture:
|
||||
- in pivot, from (1, 0);
|
||||
- in records, from (1, 2), the West row's Cost column.
|
||||
|
||||
Every cell must be unchanged. Both cases failed before the fix, clearing
|
||||
(North, Revenue). The records case was confirmed on its own, because the loop
|
||||
stops at the first failure. **Rival:** the editing keys bound through the
|
||||
welding `.commit` again fails both.
|
||||
|
||||
### `typing_after_the_reopen_still_writes`
|
||||
|
||||
After `i` and Enter from (1, 0), the reopened editor must hold `"2"`, the value
|
||||
of (North, Revenue). Typing `5` and pressing Enter must then store `25`. Before
|
||||
the fix the buffer was `""`, so the typed digit replaced the value instead of
|
||||
extending it.
|
||||
|
||||
## `ui/project.rs` — the totals strip (improvise-wm20)
|
||||
|
||||
### a_ratio_columns_total_is_the_ratio_of_the_totalled_inputs
|
||||
|
||||
@@ -1078,3 +1078,67 @@ the 5.1 control runs past the version check into `check_shebangs` and
|
||||
`check_shebangs || rc=1`, where `set -e` is off. A missing `head` makes every
|
||||
first line empty, and the shebang check then passes without checking
|
||||
anything. That is a defect in `checkers.sh` and is not fixed here.
|
||||
|
||||
### The pr-reviews watchdog resolves its tools from PATH — `scripts/pr-reviews-test.sh` (improvise-3uow)
|
||||
|
||||
**What broke.** The suite hard-coded `/bin/sleep` for the watchdog in
|
||||
`bounded_reviews` and `/bin/date` for the fake clock's pass-through. On the
|
||||
NixOS runners `/bin` holds only `sh`. With no `/bin/sleep`, the watchdog exits
|
||||
at once. `wait -n` then sees it finish before `pr-reviews`, and every bounded
|
||||
call reports 124 ("killed after 20s"). CI never showed this, because the kb job
|
||||
stopped one step earlier on the section-5b failure (#296).
|
||||
|
||||
**Reproduced** by pointing both paths at `/nonexistent/`. The result was 0
|
||||
passed and 25 failed, every `--wait` case exiting 124: the same shape the
|
||||
reviewer simulated.
|
||||
|
||||
**Fix.** `REAL_SLEEP` comes from `type -P`, before any case puts its stub
|
||||
`sleep` first on PATH. `type -P` returns a path even if `sleep` has been loaded
|
||||
as a builtin. The fake clock's `/bin/date` pass-through was deleted rather than
|
||||
re-pointed. `pr-reviews` only ever calls `date +%s`, so the branch never ran
|
||||
(found by Copilot on #297). The fake now refuses any other arguments with
|
||||
exit 2, so a new `date` call in `pr-reviews` shows up as a failure instead of
|
||||
untested code.
|
||||
|
||||
**Rivals:**
|
||||
- **`REAL_SLEEP=/nonexistent/sleep`** fails the whole suite, 0 of 25, so the
|
||||
resolved path is the one the watchdog runs.
|
||||
- **`pr-reviews` changed to call `date -u +%s`** fails "a review arriving on
|
||||
the third poll is seen" and "--wait keeps polling until its deadline" (23 of
|
||||
25 pass). The fake's refusal is therefore visible, not swallowed.
|
||||
|
||||
**Scope sweep.** `grep -rnE '/(usr/)?bin/[a-z]'` over `scripts/`, `web/*.sh`,
|
||||
`harness/*.sh`, `harness/tests` and `.github/workflows` finds only `#!/bin/sh`
|
||||
shebangs, and `/bin/sh` exists on NixOS. It also found a
|
||||
`${PATH:-/usr/bin:/bin}` fallback in `checkers.sh`, which is now removed (next
|
||||
entry).
|
||||
|
||||
### The bash floor and its self-tests are retired — `scripts/checkers.sh`
|
||||
|
||||
**The ruling.** The user ruled that CI should not care which version of bash or
|
||||
of the standard utilities it runs, and that CI jobs should use whatever the
|
||||
`.#ci` dev shell provides. So `checkers.sh` no longer pins a bash:
|
||||
`find_modern_bash`, the 5.1 floor (improvise-fd0j) and exit 3 are gone. Suites
|
||||
run under `$BASH`, the bash running the runner, with `PATH` unchanged. The
|
||||
reasoning is in `context/code/tooling.md` §5.
|
||||
|
||||
**Tests removed.** Three sections of `checkers-test.sh` were removed. They
|
||||
existed only to pin the floor, and the entries above that cite them describe
|
||||
cases that no longer exist:
|
||||
- Section 2, "a bash-3.2-only construct fails under the runner's pinned bash".
|
||||
It asserted a bash-5 behaviour, so its result depended on the bash version.
|
||||
- Section 5, "an old bash with no nix is exit 3".
|
||||
- Section 5b, "a bash 5.0 with no nix is exit 3, not 5.x" and "a bash 5.1 is
|
||||
accepted", together with the `utilbin` that #296 added for them.
|
||||
|
||||
The suite keeps 10 cases: environment scrubbing (with its direct-run pair), the
|
||||
shebang rule (with its control), fail and pass, unknown suite, and `--list`.
|
||||
|
||||
**Kept deliberately.** The environment allowlist. That is what the
|
||||
`set -u`/`$name` evidence above earns, and it does not depend on any version.
|
||||
|
||||
**Also removed with the ruling.** `scripts/kb`'s `stat -f` fallback: `kb`
|
||||
already requires the dev shell for `tpl`, and the dev shell's `stat` is GNU.
|
||||
From `ci.yml`, the toolchain version probes ("the dev shell supplies the pinned
|
||||
web toolchain", "Playwright matches the dev shell's browsers") and every
|
||||
`shell: bash` override, so every step runs in `nix develop .#ci`.
|
||||
|
||||
@@ -7,7 +7,7 @@ use crate::ui::effect::{self, Effect};
|
||||
use crate::view::RecordCol;
|
||||
use improvise_protocol::{OpenedEditor, untouched};
|
||||
|
||||
use super::core::{Cmd, CmdContext, read_buffer};
|
||||
use super::core::{Cmd, CmdContext, read_buffer, write_buffer};
|
||||
use super::grid::AddRecordRow;
|
||||
use super::navigation::{CursorState, EnterAdvance, viewport_effects};
|
||||
|
||||
@@ -830,6 +830,72 @@ mod tests {
|
||||
"the reopened editor did not capture the cell it landed on"
|
||||
);
|
||||
}
|
||||
|
||||
/// Every cell of the 2×2 fixture, by its key.
|
||||
fn every_cell(app: &crate::ui::app::App) -> Vec<Option<CellValue>> {
|
||||
use crate::model::cell::CellKey;
|
||||
let mut cells = Vec::new();
|
||||
for region in ["North", "West"] {
|
||||
for measure in ["Cost", "Revenue"] {
|
||||
let key = CellKey::new(vec![
|
||||
("Region".into(), region.into()),
|
||||
("_Measure".into(), measure.into()),
|
||||
]);
|
||||
cells.push(app.model_state.workbook.model.get_cell(&key).cloned());
|
||||
}
|
||||
}
|
||||
cells
|
||||
}
|
||||
|
||||
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn enter_enter_leaves_every_cell_unchanged() {
|
||||
use improvise_protocol::key::KeyCode;
|
||||
for (records, start) in [(false, (1, 0)), (true, (1, 2))] {
|
||||
let mut app = grid_holding(CellValue::Number(4.0), records);
|
||||
if records {
|
||||
app.view_state.mode = AppMode::RecordsNormal;
|
||||
}
|
||||
app.cursor_mut().selected = start;
|
||||
let before = every_cell(&app);
|
||||
press(&mut app, KeyCode::Char('i'));
|
||||
press(&mut app, KeyCode::Enter);
|
||||
let reopened = app.view_state.buffers.get("edit").cloned();
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(
|
||||
every_cell(&app),
|
||||
before,
|
||||
"records={records}: i, Enter, Enter changed a cell (the reopened editor held {reopened:?})"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The reopened editor shows the cell it landed on, so typing extends it.
|
||||
/// BUG (improvise-fy9o) — see `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn typing_after_the_reopen_still_writes() {
|
||||
use crate::model::cell::CellKey;
|
||||
use improvise_protocol::key::KeyCode;
|
||||
let mut app = grid_holding(CellValue::Number(4.0), false);
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
press(&mut app, KeyCode::Char('i'));
|
||||
press(&mut app, KeyCode::Enter);
|
||||
assert_eq!(
|
||||
app.view_state.buffers.get("edit").map(String::as_str),
|
||||
Some("2"),
|
||||
"the editor reopened on (North, Revenue) is not seeded with its value"
|
||||
);
|
||||
press(&mut app, KeyCode::Char('5'));
|
||||
press(&mut app, KeyCode::Enter);
|
||||
let landed = CellKey::new(vec![
|
||||
("Region".into(), "North".into()),
|
||||
("_Measure".into(), "Revenue".into()),
|
||||
]);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.get_cell(&landed),
|
||||
Some(&CellValue::Number(25.0))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Commit commands (mode-specific buffer consumers) ────────────────────────
|
||||
@@ -976,6 +1042,11 @@ impl Cmd for CommitAndAdvance {
|
||||
// Drop to normal **before** the commit, which can abort: tidy-up goes where
|
||||
// an abort cannot reach it (improvise-2zq, gotcha 21).
|
||||
effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
|
||||
// This command owns its buffer's clear, at the head, so the re-entry
|
||||
// below re-seeds after it rather than being blanked (improvise-fy9o).
|
||||
if let Some(mb) = self.edit_mode.minibuffer() {
|
||||
effects.push(write_buffer(&mb.buffer_key, String::new()));
|
||||
}
|
||||
// An untouched editor writes nothing; everything around the write still runs.
|
||||
let writes = !untouched(self.opened.as_ref(), &self.value);
|
||||
if writes {
|
||||
|
||||
@@ -753,6 +753,22 @@ mod tests {
|
||||
}
|
||||
|
||||
/// The buffer `append-char` is bound to in `mode`'s keymap.
|
||||
/// The effects `name` builds in `mode` with `buffer` holding text, as Debug.
|
||||
fn clears_itself(mode: &AppMode, name: &str, args: &[String], buffer: &str) -> String {
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
let wb = two_cat_model();
|
||||
let layout = make_layout(&wb);
|
||||
let reg = make_registry();
|
||||
let bufs: HashMap<String, String> = [(buffer.to_string(), "typed".to_string())].into();
|
||||
let mut ctx = make_ctx(&wb, &layout, ®);
|
||||
ctx.mode = mode;
|
||||
ctx.buffers = &bufs;
|
||||
let cmd = reg
|
||||
.interactive(name, args, &ctx)
|
||||
.unwrap_or_else(|e| panic!("{name} {args:?} does not build: {e}"));
|
||||
effects_debug(&cmd.execute(&ctx))
|
||||
}
|
||||
|
||||
fn append_target(ks: &KeymapSet, mode: &AppMode) -> String {
|
||||
let key = ModeKey::from_app_mode(mode, false).expect("a text-entry mode key");
|
||||
let km = ks.maps().get(&key).expect("every mode has a keymap");
|
||||
@@ -827,15 +843,26 @@ mod tests {
|
||||
let km = ks.maps().get(&mode).unwrap();
|
||||
|
||||
for key in exits {
|
||||
let Some(Binding::Sequence(steps)) = km.lookup(*key, none) else {
|
||||
panic!("{mode:?}: {key:?} should be a commit/exit sequence");
|
||||
};
|
||||
assert!(
|
||||
steps
|
||||
.iter()
|
||||
.any(|(n, a)| *n == "clear-buffer" && a.as_slice() == [buffer.to_string()]),
|
||||
"{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}"
|
||||
);
|
||||
match km.lookup(*key, none) {
|
||||
Some(Binding::Sequence(steps)) => {
|
||||
assert!(
|
||||
steps.iter().any(|(n, a)| *n == "clear-buffer"
|
||||
&& a.as_slice() == [buffer.to_string()]),
|
||||
"{mode:?}: {key:?} must clear \"{buffer}\", got {steps:?}"
|
||||
)
|
||||
}
|
||||
// A reopening commit owns its clear (improvise-fy9o): ask the command.
|
||||
Some(Binding::Cmd { name, args }) => {
|
||||
let dbg = clears_itself(&app_mode, name, args, &buffer);
|
||||
assert!(
|
||||
dbg.contains(&format!(
|
||||
"SetBuffer {{ name: \"{buffer}\", value: \"\" }}"
|
||||
)),
|
||||
"{mode:?}: {key:?} runs {name} alone, which must clear \"{buffer}\": {dbg}"
|
||||
);
|
||||
}
|
||||
other => panic!("{mode:?}: {key:?} should commit or exit, got {other:?}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -113,6 +113,8 @@ impl ClientState {
|
||||
}
|
||||
}
|
||||
self.settle_sent();
|
||||
self.settle_editor();
|
||||
self.revision += 1;
|
||||
}
|
||||
|
||||
/// The host applied **none** of the oldest awaited frame (ADR 001 clause
|
||||
@@ -175,6 +177,23 @@ impl ClientState {
|
||||
self.view.buffers.insert(key, seed.clone());
|
||||
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
|
||||
}
|
||||
self.settle_awaited_seed();
|
||||
}
|
||||
|
||||
/// An editor opened after a write takes its seed from the grid the host
|
||||
/// answered with, never from the cache the write made stale (improvise-3uzg).
|
||||
fn settle_awaited_seed(&mut self) {
|
||||
let waiting = self.view.mode.is_editing()
|
||||
&& self
|
||||
.view
|
||||
.opened_editor
|
||||
.as_ref()
|
||||
.is_some_and(|opened| opened.awaiting && !opened.touched);
|
||||
let answered = !self.in_flight.iter().any(|sent| sent.awaited);
|
||||
if let (true, true, Some(seed)) = (waiting, answered, crate::resolve::edit_seed(self)) {
|
||||
self.view.buffers.insert("edit".to_string(), seed.clone());
|
||||
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
|
||||
}
|
||||
}
|
||||
|
||||
/// The widget dispatch: a projection names the widget it is addressed to.
|
||||
@@ -398,6 +417,102 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A write, then the editor its advance reopens with the seed still to come.
|
||||
fn write_then_awaiting_editor() -> Vec<Command> {
|
||||
let mut frame = commit_like("written");
|
||||
frame.extend([
|
||||
Command::SetBuffer {
|
||||
name: "edit".into(),
|
||||
value: String::new(),
|
||||
},
|
||||
Command::SetOpenedEditor(Some(crate::OpenedEditor::awaiting())),
|
||||
Command::ChangeMode(crate::AppMode::editing()),
|
||||
]);
|
||||
frame
|
||||
}
|
||||
|
||||
/// One row, one column, the cell's stored text `edit_text`.
|
||||
fn answered_with(state: &mut ClientState, edit_text: &str) {
|
||||
let mut cell = CellDisplay::number(edit_text);
|
||||
cell.edit_text = Some(edit_text.into());
|
||||
for cmd in [
|
||||
GridUpdate::RowFrame(AxisFrame::flat(["North"])),
|
||||
GridUpdate::ColFrame(AxisFrame::flat(["Revenue"])),
|
||||
GridUpdate::Cells(vec![((0, 0), cell)]),
|
||||
] {
|
||||
state.fold(&Command::Project(Projection::Grid(cmd)));
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-3uzg) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn an_awaited_seed_waits_for_the_answer_and_never_overwrites_typing() {
|
||||
let mut state = ClientState::default();
|
||||
answered_with(&mut state, "2");
|
||||
state.fold_outgoing(&write_then_awaiting_editor());
|
||||
assert_eq!(state.view.buffers.get("edit").map(String::as_str), Some(""));
|
||||
answered_with(&mut state, "3");
|
||||
assert_eq!(
|
||||
state.view.buffers.get("edit").map(String::as_str),
|
||||
Some(""),
|
||||
"seeded while the write was still unanswered"
|
||||
);
|
||||
state.accepted();
|
||||
assert_eq!(
|
||||
state.view.buffers.get("edit").map(String::as_str),
|
||||
Some("3"),
|
||||
"the answered editor must open on the host's grid"
|
||||
);
|
||||
assert_eq!(
|
||||
state.view.opened_editor,
|
||||
Some(crate::OpenedEditor::on("3".into()))
|
||||
);
|
||||
|
||||
let mut typed_ahead = ClientState::default();
|
||||
answered_with(&mut typed_ahead, "2");
|
||||
typed_ahead.fold_outgoing(&write_then_awaiting_editor());
|
||||
typed_ahead.fold_outgoing(&[
|
||||
Command::SetBuffer {
|
||||
name: "edit".into(),
|
||||
value: "5".into(),
|
||||
},
|
||||
Command::SetOpenedEditor(crate::resolve::edited(
|
||||
typed_ahead.view.opened_editor.as_ref(),
|
||||
"edit",
|
||||
)),
|
||||
]);
|
||||
answered_with(&mut typed_ahead, "3");
|
||||
typed_ahead.accepted();
|
||||
assert_eq!(
|
||||
typed_ahead.view.buffers.get("edit").map(String::as_str),
|
||||
Some("5"),
|
||||
"a seed arriving after the user typed must not overwrite the typing"
|
||||
);
|
||||
}
|
||||
|
||||
/// BUG (improvise-4cb5) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn an_awaited_seed_arriving_after_escape_leaves_the_buffer_empty() {
|
||||
let mut state = ClientState::default();
|
||||
answered_with(&mut state, "2");
|
||||
state.fold_outgoing(&write_then_awaiting_editor());
|
||||
state.fold_outgoing(&[
|
||||
Command::SetBuffer {
|
||||
name: "edit".into(),
|
||||
value: String::new(),
|
||||
},
|
||||
Command::ChangeMode(crate::AppMode::Normal),
|
||||
]);
|
||||
answered_with(&mut state, "3");
|
||||
state.accepted();
|
||||
assert_eq!(state.view.mode, crate::AppMode::Normal);
|
||||
assert_eq!(
|
||||
state.view.buffers.get("edit").map(String::as_str),
|
||||
Some(""),
|
||||
"the late seed was written into the buffer of an editor Esc had closed"
|
||||
);
|
||||
}
|
||||
|
||||
/// A frame the host does not answer must not consume an answer, or the
|
||||
/// queue addresses the wrong frame. Evidence file.
|
||||
#[test]
|
||||
|
||||
@@ -347,6 +347,13 @@ impl TextEntry {
|
||||
self
|
||||
}
|
||||
|
||||
/// Bind `key` to a commit that clears the buffer **itself**, first, because
|
||||
/// it reopens an editor a trailing clear would blank (improvise-fy9o).
|
||||
pub fn commit_reopening(mut self, key: KeyCode, cmd: &'static str, args: Vec<String>) -> Self {
|
||||
self.km.bind_args(key, KeyModifiers::NONE, cmd, args);
|
||||
self
|
||||
}
|
||||
|
||||
/// Replace a spine binding — used only by CommandMode, whose Backspace
|
||||
/// exits the mode on an empty buffer instead of popping a character.
|
||||
pub fn rebind(mut self, key: KeyCode, cmd: &'static str) -> Self {
|
||||
@@ -853,8 +860,8 @@ impl Keymaps {
|
||||
// edit-mode as an argument, so the command stays mode-agnostic. ──
|
||||
let ed = Arc::new(
|
||||
TextEntry::new(&AppMode::editing(), "normal", global.clone())
|
||||
.commit(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
|
||||
.commit(
|
||||
.commit_reopening(KeyCode::Enter, "commit-cell-edit", vec!["editing".into()])
|
||||
.commit_reopening(
|
||||
KeyCode::Tab,
|
||||
"commit-and-advance-right",
|
||||
vec!["editing".into()],
|
||||
@@ -869,12 +876,12 @@ impl Keymaps {
|
||||
ModeKey::RecordsEditing,
|
||||
Arc::new(
|
||||
TextEntry::new(&AppMode::records_editing(), "records-normal", ed)
|
||||
.commit(
|
||||
.commit_reopening(
|
||||
KeyCode::Enter,
|
||||
"commit-cell-edit",
|
||||
vec!["records-editing".into()],
|
||||
)
|
||||
.commit(
|
||||
.commit_reopening(
|
||||
KeyCode::Tab,
|
||||
"commit-and-advance-right",
|
||||
vec!["records-editing".into()],
|
||||
|
||||
@@ -809,7 +809,11 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
|
||||
match cell.id.as_ref()? {
|
||||
// `edit_text`, never `text`: the drawn text has been through
|
||||
// `format_f64`. `tests/lockstep.rs` found exactly that.
|
||||
CellId::Record(_) => Some(opened_on(cell.edit_text.clone().unwrap_or_default(), mode)),
|
||||
CellId::Record(_) => Some(opened_on(
|
||||
state,
|
||||
cell.edit_text.clone().unwrap_or_default(),
|
||||
mode,
|
||||
)),
|
||||
CellId::Aggregate(_) => Some(vec![Command::SetStatus(format!(
|
||||
"an aggregate is edited by drilling in — {}",
|
||||
not_yet("drill")
|
||||
@@ -822,9 +826,18 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Open the cell editor on `seed`: the buffer, what it captured, then the mode.
|
||||
/// `EditOrDrill`'s and `EnterEditAtCursor`'s shared shape (improvise-ixxx).
|
||||
fn opened_on(seed: String, mode: AppMode) -> Vec<Command> {
|
||||
/// Open the cell editor on `seed`, or await it while any write is unanswered: the
|
||||
/// cache predates that write (improvise-mdfc/p6v6, `protocol.md` §3).
|
||||
fn opened_on(state: &ClientState, seed: String, mode: AppMode) -> Vec<Command> {
|
||||
match state.in_flight.iter().any(|sent| sent.awaited) {
|
||||
true => awaiting_seed(mode),
|
||||
false => seeded(seed, mode),
|
||||
}
|
||||
}
|
||||
|
||||
/// The buffer, what the editor captured, then the mode: `EditOrDrill`'s and
|
||||
/// `EnterEditAtCursor`'s shared shape (improvise-ixxx).
|
||||
fn seeded(seed: String, mode: AppMode) -> Vec<Command> {
|
||||
vec![
|
||||
Command::SetBuffer {
|
||||
name: "edit".to_string(),
|
||||
@@ -943,10 +956,20 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
|
||||
}
|
||||
let mut commands = written(state, id, &text)?;
|
||||
commands.push(Command::ChangeMode(AppMode::Normal));
|
||||
commands.extend(advanced(state, edit_mode, advance)?);
|
||||
commands.extend(cleared(&edit_mode));
|
||||
let writes = !untouched(state.view.opened_editor.as_ref(), &text);
|
||||
commands.extend(advanced(state, edit_mode, advance, writes)?);
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// The commit's own clear of `edit_mode`'s buffer, at its head: the keymap
|
||||
/// welds none to these keys, so a re-seed survives (improvise-fy9o).
|
||||
fn cleared(edit_mode: &AppMode) -> Option<Command> {
|
||||
edit_mode
|
||||
.minibuffer()
|
||||
.map(|mb| write_buffer(&mb.buffer_key, String::new()))
|
||||
}
|
||||
|
||||
/// The write a commit makes: none when the buffer is the seed the editor
|
||||
/// captured as it opened, and only the write is skipped (improvise-ixxx).
|
||||
fn written(state: &ClientState, id: &CellId, text: &str) -> Option<Vec<Command>> {
|
||||
@@ -971,6 +994,7 @@ fn create_column(
|
||||
// `exit_mode_for`: the tidy-up goes FIRST, where an aborting
|
||||
// `AddCategory` cannot reach it (gotcha 21).
|
||||
let mut commands = vec![Command::ChangeMode(AppMode::Normal)];
|
||||
commands.extend(cleared(&edit_mode));
|
||||
let name = text.trim();
|
||||
if !name.is_empty() {
|
||||
commands.push(model(
|
||||
@@ -991,13 +1015,18 @@ fn create_column(
|
||||
if matches!(kind, CreateKind::Category) {
|
||||
return Some(commands);
|
||||
}
|
||||
commands.extend(advanced(state, edit_mode, advance)?);
|
||||
commands.extend(advanced(state, edit_mode, advance, !name.is_empty())?);
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// The advance and the re-entry `CommitAndAdvance` ends in. `None` where the
|
||||
/// host would emit `AddRecordRow`, which this client cannot name (gotcha 1).
|
||||
fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> {
|
||||
fn advanced(
|
||||
state: &ClientState,
|
||||
edit_mode: AppMode,
|
||||
advance: Advance,
|
||||
writes: bool,
|
||||
) -> Option<Vec<Command>> {
|
||||
let GridExtent { rows, cols } = state.grid.extent;
|
||||
let row_max = rows.saturating_sub(1);
|
||||
let col_max = cols.saturating_sub(1);
|
||||
@@ -1019,11 +1048,25 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
|
||||
return Some(Vec::new());
|
||||
};
|
||||
let mut commands = scrolled_to(state, nr, nc);
|
||||
// The new cell is a different cell, so a stale cache is correct here.
|
||||
commands.extend(reopened_at(state, nr, nc, edit_mode));
|
||||
// This frame's own write makes the cache stale too; `opened_on` answers for
|
||||
// earlier ones (improvise-3uzg/mdfc/p6v6, `protocol.md` §3).
|
||||
commands.extend(reopened_at(state, nr, nc, edit_mode, writes));
|
||||
Some(commands)
|
||||
}
|
||||
|
||||
/// Open the cell editor with its seed still to come: the client settles it from
|
||||
/// the host's answer, once nothing this client sent is unanswered (improvise-3uzg).
|
||||
fn awaiting_seed(mode: AppMode) -> Vec<Command> {
|
||||
vec![
|
||||
Command::SetBuffer {
|
||||
name: EDIT_BUFFER.to_string(),
|
||||
value: String::new(),
|
||||
},
|
||||
Command::SetOpenedEditor(Some(OpenedEditor::awaiting())),
|
||||
Command::ChangeMode(mode),
|
||||
]
|
||||
}
|
||||
|
||||
/// The surrogate naming the record under the cursor, or `None` when the
|
||||
/// cursor is not on a writable cell.
|
||||
fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
|
||||
@@ -1058,13 +1101,20 @@ fn cell_at(state: &ClientState, row: usize, col: usize) -> Option<&crate::CellDi
|
||||
|
||||
/// `EnterEditAtCursor`: re-open the editor where the advance landed, unless `i`
|
||||
/// would decline that cell. The host asks `uneditable`; this asks the same id.
|
||||
fn reopened_at(state: &ClientState, row: usize, col: usize, mode: AppMode) -> Vec<Command> {
|
||||
fn reopened_at(
|
||||
state: &ClientState,
|
||||
row: usize,
|
||||
col: usize,
|
||||
mode: AppMode,
|
||||
writes: bool,
|
||||
) -> Vec<Command> {
|
||||
match cell_at(state, row, col).and_then(|cell| cell.id.as_ref()) {
|
||||
Some(CellId::Aggregate(_)) => vec![Command::SetStatus(AGGREGATE_DECLINE.to_string())],
|
||||
Some(CellId::Derived { target }) => vec![Command::SetStatus(derived_decline(target))],
|
||||
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => {
|
||||
opened_on(seed_at(state, row, col), mode)
|
||||
}
|
||||
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => match writes {
|
||||
true => awaiting_seed(mode),
|
||||
false => opened_on(state, seed_at(state, row, col), mode),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1084,6 +1134,9 @@ pub struct OpenedEditor {
|
||||
pub seed: String,
|
||||
/// Whether the user has edited the buffer since it opened (improvise-2054).
|
||||
pub touched: bool,
|
||||
/// The seed is still to come from the host's answer (improvise-3uzg). Not
|
||||
/// `serde(default)`: `false` is a claim — `protocol.md` §3, improvise-mgis.
|
||||
pub awaiting: bool,
|
||||
}
|
||||
|
||||
impl OpenedEditor {
|
||||
@@ -1092,6 +1145,16 @@ impl OpenedEditor {
|
||||
Self {
|
||||
seed,
|
||||
touched: false,
|
||||
awaiting: false,
|
||||
}
|
||||
}
|
||||
|
||||
/// An editor opened before its seed is known: empty until it settles.
|
||||
pub fn awaiting() -> Self {
|
||||
Self {
|
||||
seed: String::new(),
|
||||
touched: false,
|
||||
awaiting: true,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -3494,18 +3557,31 @@ mod tests {
|
||||
}
|
||||
);
|
||||
assert_eq!(commands[1], Command::ChangeMode(AppMode::Normal));
|
||||
assert_eq!(commands[2], Command::SetSelected { row: 1, col: 0 });
|
||||
assert!(
|
||||
commands.contains(&Command::ChangeMode(AppMode::editing())),
|
||||
"the advance re-enters editing on the cell it landed on: {commands:?}"
|
||||
);
|
||||
// The commit owns its clear, at the head (improvise-fy9o).
|
||||
assert_eq!(
|
||||
commands.last(),
|
||||
Some(&Command::SetBuffer {
|
||||
commands[2],
|
||||
Command::SetBuffer {
|
||||
name: "edit".into(),
|
||||
value: String::new(),
|
||||
}),
|
||||
"the keymap welds `clear-buffer` to every commit: {commands:?}"
|
||||
}
|
||||
);
|
||||
assert_eq!(commands[3], Command::SetSelected { row: 1, col: 0 });
|
||||
assert_eq!(
|
||||
commands.last(),
|
||||
Some(&Command::ChangeMode(AppMode::editing())),
|
||||
"the advance re-enters editing on the cell it landed on: {commands:?}"
|
||||
);
|
||||
assert!(
|
||||
commands.contains(&Command::SetOpenedEditor(Some(OpenedEditor::awaiting()))),
|
||||
"after a write the reopened editor awaits the host's seed: {commands:?}"
|
||||
);
|
||||
for command in &commands {
|
||||
client.fold(command);
|
||||
}
|
||||
assert_eq!(
|
||||
client.view.buffers.get("edit").map(String::as_str),
|
||||
Some("900"),
|
||||
"an answered client seeds the reopened editor from the cell it landed on"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -4145,3 +4221,33 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod opened_editor_wire_tests {
|
||||
use super::OpenedEditor;
|
||||
use crate::{ClientFrame, Command};
|
||||
|
||||
/// improvise-mgis: `awaiting: false` is the claim that the seed is settled,
|
||||
/// so a frame that dropped it is refused. `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_frame_whose_editor_lost_its_awaiting_flag_does_not_parse() {
|
||||
let frame = ClientFrame::Commands(vec![Command::SetOpenedEditor(Some(
|
||||
OpenedEditor::awaiting(),
|
||||
))]);
|
||||
let mut json = serde_json::to_value(&frame).expect("a ClientFrame serializes");
|
||||
let opened = json
|
||||
.pointer_mut("/Commands/0/SetOpenedEditor")
|
||||
.and_then(serde_json::Value::as_object_mut)
|
||||
.expect("the editor is where this test expects it");
|
||||
assert!(
|
||||
opened.remove("awaiting").is_some() && opened.contains_key("seed"),
|
||||
"the field must have been there, and the rest of the editor must remain"
|
||||
);
|
||||
let err = serde_json::from_value::<ClientFrame>(json)
|
||||
.expect_err("an editor that states no awaiting flag states no claim");
|
||||
assert!(
|
||||
err.to_string().contains("missing field `awaiting`"),
|
||||
"it must fail as the MISSING FIELD, not as broken JSON: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1727,6 +1727,45 @@ fn a_locally_committed_creating_column_agrees_with_the_host() {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-5wai: `+cat` reopens nothing, so its commit's own clear is all
|
||||
/// that empties the buffer, on both ends. Evidence file.
|
||||
#[test]
|
||||
fn a_category_committed_at_plus_cat_leaves_no_buffer_behind_on_both_ends() {
|
||||
let (mut app, mut twin) = records_pair();
|
||||
let client = furnished_client(&app, (12, 12));
|
||||
let at = creating_cell(&client, improvise_protocol::CreateKind::Category);
|
||||
let mut client = client;
|
||||
typing_at(&mut app, &mut twin, &mut client, at, "Season");
|
||||
client = furnished_client(&app, (12, 12));
|
||||
client.view = app.view_state.clone();
|
||||
assert_eq!(
|
||||
read_edit(&client.view.buffers),
|
||||
"Season",
|
||||
"nothing was typed, so this proves nothing"
|
||||
);
|
||||
|
||||
both_ends(&mut app, &mut twin, &mut client, "Enter");
|
||||
|
||||
assert!(
|
||||
document_text(&app).contains("Season"),
|
||||
"the commit did not land"
|
||||
);
|
||||
assert_eq!(
|
||||
read_edit(&app.view_state.buffers),
|
||||
"",
|
||||
"the host left the name behind"
|
||||
);
|
||||
assert_eq!(
|
||||
read_edit(&client.view.buffers),
|
||||
"",
|
||||
"the browser left the name behind"
|
||||
);
|
||||
}
|
||||
|
||||
fn read_edit(buffers: &std::collections::HashMap<String, String>) -> &str {
|
||||
buffers.get("edit").map(String::as_str).unwrap_or("")
|
||||
}
|
||||
|
||||
/// The twin of the commit test above, for the door rather than the exit
|
||||
/// (improvise-bd9o). Evidence file.
|
||||
#[test]
|
||||
@@ -2230,6 +2269,179 @@ fn an_untouched_browser_edit_of_a_misread_value_writes_nothing() {
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn enter_enter_leaves_every_cell_unchanged_on_both_ends() {
|
||||
let every_cell = |app: &App| -> Vec<Option<improvise_protocol::CellValue>> {
|
||||
[
|
||||
("North", "Cost"),
|
||||
("North", "Revenue"),
|
||||
("South", "Cost"),
|
||||
("South", "Revenue"),
|
||||
]
|
||||
.iter()
|
||||
.map(|(r, m)| {
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure(r, m))
|
||||
.cloned()
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (1, 0);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (1, 0);
|
||||
let before = every_cell(&client_host);
|
||||
let mut client = client_ready_to_edit(&client_host, (2, 2));
|
||||
for key in ["i", "Enter", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("{key}: the client must answer this key locally");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(&mut client_host, &cmds);
|
||||
reduce_key(&mut key_host, event);
|
||||
assert_eq!(
|
||||
client.view.buffers, key_host.view_state.buffers,
|
||||
"{key}: client and host disagree about the buffers"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
every_cell(&client_host),
|
||||
before,
|
||||
"the browser's Enter, Enter wrote"
|
||||
);
|
||||
assert_eq!(
|
||||
every_cell(&key_host),
|
||||
before,
|
||||
"the host's Enter, Enter wrote"
|
||||
);
|
||||
}
|
||||
|
||||
/// A commit that reopens nothing still leaves no text behind, on either end:
|
||||
/// the clear moved from the keymap into the commit (improvise-fy9o).
|
||||
#[test]
|
||||
fn a_commit_at_the_corner_leaves_no_buffer_behind_on_both_ends() {
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
assert_eq!(
|
||||
key_host.cursor().selected,
|
||||
(1, 1),
|
||||
"the fixture is not at the corner"
|
||||
);
|
||||
let mut client = client_ready_to_edit(&client_host, (2, 2));
|
||||
for key in ["i", "9", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("{key}: the client must answer this key locally");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(&mut client_host, &cmds);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
assert!(
|
||||
!key_host.view_state.mode.is_editing(),
|
||||
"the corner must not reopen"
|
||||
);
|
||||
for (end, buffers) in [
|
||||
("browser", &client.view.buffers),
|
||||
("host", &key_host.view_state.buffers),
|
||||
] {
|
||||
assert_eq!(
|
||||
buffers.get("edit").map(String::as_str),
|
||||
Some(""),
|
||||
"{end}: the committed text was left in the buffer"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-3tt4) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn tab_at_the_rightmost_column_reopens_on_the_value_just_written() {
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (0, 1);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (0, 1);
|
||||
let mut client = client_ready_to_edit(&client_host, (2, 2));
|
||||
for key in ["i", "9", "Tab", "5", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, (2, 2));
|
||||
reduce_key(&mut key_host, event);
|
||||
assert_eq!(
|
||||
client.view.buffers, key_host.view_state.buffers,
|
||||
"{key}: client and host disagree about the buffers"
|
||||
);
|
||||
}
|
||||
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", "Revenue")),
|
||||
Some(&improvise_protocol::CellValue::Number(295.0)),
|
||||
"{end}: the reopened editor did not extend the value Tab had just written"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The reopened seed is the STORED value's edit text, not the typed string:
|
||||
/// `007` is stored as 7 on both ends (improvise-3tt4). Evidence file.
|
||||
#[test]
|
||||
fn tab_at_the_rightmost_column_reopens_on_the_stored_spelling() {
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (0, 1);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (0, 1);
|
||||
let mut client = client_ready_to_edit(&client_host, (2, 2));
|
||||
for key in ["i", "Backspace", "0", "0", "7", "Tab"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, (2, 2));
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
assert_eq!(
|
||||
key_host.view_state.buffers.get("edit").map(String::as_str),
|
||||
Some("7")
|
||||
);
|
||||
assert_eq!(client.view.buffers, key_host.view_state.buffers);
|
||||
}
|
||||
|
||||
/// BUG (improvise-fy9o) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn typing_into_the_reopened_editor_extends_the_cell_on_both_ends() {
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (1, 0);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (1, 0);
|
||||
let mut client = client_ready_to_edit(&client_host, (2, 2));
|
||||
for key in ["i", "Enter", "5", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
|
||||
panic!("{key}: the client must answer this key locally");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
apply_on(&mut client_host, &cmds);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", "Revenue")),
|
||||
Some(&improvise_protocol::CellValue::Number(25.0)),
|
||||
"{end}: the digit typed into the reopened editor did not extend its seed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// `i` then `Enter` on (South, Revenue) holding `value`, through the resolver
|
||||
/// and a real host: the seed is the stored value's, and the cell is unchanged.
|
||||
fn untouched_browser_edit(value: improvise_protocol::CellValue) {
|
||||
@@ -2315,6 +2527,212 @@ fn every_projected_edit_text_commits_back_to_its_value() {
|
||||
}
|
||||
}
|
||||
|
||||
/// North/South/West = 1/2/3 on one measure, with `prune_empty` on: emptying a
|
||||
/// row's only value takes the row away and renumbers the rest.
|
||||
fn pruned_host() -> App {
|
||||
let mut wb = Workbook::new("Test");
|
||||
wb.add_category("Region").unwrap();
|
||||
for region in ["North", "South", "West"] {
|
||||
wb.add_item("Region", region).unwrap();
|
||||
}
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.active_view_mut().set_axis("Region", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Measure", Axis::Column);
|
||||
wb.active_view_mut().prune_empty = true;
|
||||
for (region, value) in [("North", 1.0), ("South", 2.0), ("West", 3.0)] {
|
||||
wb.model.set_cell(
|
||||
region_measure(region, "Revenue"),
|
||||
improvise_protocol::CellValue::Number(value),
|
||||
);
|
||||
}
|
||||
let mut app = App::new(wb, None);
|
||||
app.term_height = 3 + 8;
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// One key the way a real browser sends it: folded as an outgoing frame, applied
|
||||
/// by the host, then answered with the host's own projection and `Accepted`.
|
||||
fn sent_and_answered(
|
||||
client: &mut ClientState,
|
||||
host: &mut App,
|
||||
event: KeyEvent,
|
||||
window: (usize, usize),
|
||||
) {
|
||||
let KeyOutcome::Local(cmds) = resolve(client, event) else {
|
||||
panic!("{event:?}: the client must answer this key locally");
|
||||
};
|
||||
client.fold_outgoing(&cmds);
|
||||
apply_on(host, &cmds);
|
||||
client.grid = client_of(host, window).grid;
|
||||
improvise_protocol::WidgetCache::apply(&mut client.basis, &published_basis(host, window));
|
||||
client.accepted();
|
||||
}
|
||||
|
||||
/// BUG (improvise-3uzg) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_commit_that_prunes_its_row_reopens_on_the_row_that_moved_up() {
|
||||
let window = (3, 3);
|
||||
let mut client_host = pruned_host();
|
||||
let mut key_host = pruned_host();
|
||||
let mut client = client_ready_to_edit(&client_host, window);
|
||||
for key in ["i", "Backspace", "Enter", "5", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, window);
|
||||
reduce_key(&mut key_host, event);
|
||||
assert_eq!(
|
||||
client.view.buffers, key_host.view_state.buffers,
|
||||
"{key}: client and host disagree about the buffers"
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
document_text(&client_host),
|
||||
document_text(&key_host),
|
||||
"the browser wrote a value built from another record's number"
|
||||
);
|
||||
assert!(
|
||||
document_text(&key_host).contains("= 35"),
|
||||
"setup: the host's reopened editor extends West's 3"
|
||||
);
|
||||
}
|
||||
|
||||
/// What the host owes a frame it has applied: its projection, its basis, and
|
||||
/// `Accepted` when the frame was awaited. Held back so the next key overtakes it.
|
||||
struct Unanswered {
|
||||
grid: improvise_protocol::GridCache,
|
||||
basis: improvise_protocol::Basis,
|
||||
awaited: bool,
|
||||
}
|
||||
|
||||
fn sent(
|
||||
client: &mut ClientState,
|
||||
host: &mut App,
|
||||
event: KeyEvent,
|
||||
window: (usize, usize),
|
||||
) -> Unanswered {
|
||||
let KeyOutcome::Local(cmds) = resolve(client, event) else {
|
||||
panic!("{event:?}: the client must answer this key locally");
|
||||
};
|
||||
client.fold_outgoing(&cmds);
|
||||
apply_on(host, &cmds);
|
||||
let awaited = cmds
|
||||
.iter()
|
||||
.any(|c| matches!(c, improvise_protocol::Command::Model { .. }));
|
||||
let (grid, basis) = (client_of(host, window).grid, published_basis(host, window));
|
||||
Unanswered {
|
||||
grid,
|
||||
basis,
|
||||
awaited,
|
||||
}
|
||||
}
|
||||
|
||||
fn answered(client: &mut ClientState, answer: Unanswered) {
|
||||
client.grid = answer.grid;
|
||||
improvise_protocol::WidgetCache::apply(&mut client.basis, &answer.basis);
|
||||
if answer.awaited {
|
||||
client.accepted();
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-mdfc) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_tab_sent_before_the_last_write_is_answered_reopens_on_that_write() {
|
||||
let window = (2, 2);
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (0, 1);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (0, 1);
|
||||
let mut client = client_ready_to_edit(&client_host, window);
|
||||
let tab = BrowserKey::new("Tab").to_key_event().unwrap();
|
||||
for key in ["i", "9"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, window);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
// Three Tabs, so a frame that writes nothing sits between the write and the last.
|
||||
let answers: Vec<Unanswered> = (0..3)
|
||||
.map(|_| sent(&mut client, &mut client_host, tab, window))
|
||||
.collect();
|
||||
let awaited: Vec<bool> = answers.iter().map(|a| a.awaited).collect();
|
||||
assert_eq!(
|
||||
awaited,
|
||||
[true, false, false],
|
||||
"setup: only the first Tab writes"
|
||||
);
|
||||
for answer in answers {
|
||||
reduce_key(&mut key_host, tab);
|
||||
answered(&mut client, answer);
|
||||
}
|
||||
assert_eq!(
|
||||
client.view.buffers, key_host.view_state.buffers,
|
||||
"the second editor opened on the cache the first write had made stale"
|
||||
);
|
||||
for key in ["5", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, window);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", "Revenue")),
|
||||
Some(&improvise_protocol::CellValue::Number(295.0)),
|
||||
"{end}: the browser overwrote the value Tab had just written"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// BUG (improvise-p6v6) — see `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn an_editor_opened_with_i_before_the_last_write_is_answered_opens_on_that_write() {
|
||||
let window = (2, 2);
|
||||
let mut client_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
client_host.cursor_mut().selected = (0, 1);
|
||||
let mut key_host = precise_host(improvise_protocol::CellValue::Number(4.0));
|
||||
key_host.cursor_mut().selected = (0, 1);
|
||||
let mut client = client_ready_to_edit(&client_host, window);
|
||||
for key in ["i", "9"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, window);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
// Tab writes 29; Esc and i reopen before its answer comes back.
|
||||
let answers: Vec<Unanswered> = ["Tab", "Escape", "i"]
|
||||
.into_iter()
|
||||
.map(|key| {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
reduce_key(&mut key_host, event);
|
||||
sent(&mut client, &mut client_host, event, window)
|
||||
})
|
||||
.collect();
|
||||
let awaited: Vec<bool> = answers.iter().map(|a| a.awaited).collect();
|
||||
assert_eq!(awaited, [true, false, false], "setup: only the Tab writes");
|
||||
answers
|
||||
.into_iter()
|
||||
.for_each(|answer| answered(&mut client, answer));
|
||||
assert_eq!(
|
||||
client.view.buffers, key_host.view_state.buffers,
|
||||
"i opened on the cache the unanswered write had made stale"
|
||||
);
|
||||
for key in ["5", "Enter"] {
|
||||
let event = BrowserKey::new(key).to_key_event().unwrap();
|
||||
sent_and_answered(&mut client, &mut client_host, event, window);
|
||||
reduce_key(&mut key_host, event);
|
||||
}
|
||||
for (end, app) in [("browser", &client_host), ("host", &key_host)] {
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.model
|
||||
.get_cell(®ion_measure("North", "Revenue")),
|
||||
Some(&improvise_protocol::CellValue::Number(295.0)),
|
||||
"{end}: the browser overwrote the value Tab had just written"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
const FORMULAS: [&str; 3] = [
|
||||
"Profit = Revenue * 2",
|
||||
"Tax = Revenue * 0.1",
|
||||
|
||||
@@ -43,7 +43,7 @@
|
||||
# behind the runners' TLS-auditing proxy those downloads fail (binaryen
|
||||
# with no fallback). On PATH, it downloads nothing. wasm-pack only takes
|
||||
# a PATH wasm-bindgen whose version equals Cargo.lock's `wasm-bindgen`,
|
||||
# so this pin moves with the lock; CI checks that it does.
|
||||
# so this pin moves with the lock: on a mismatch the sandboxed build fails.
|
||||
wasmBindgenCli = pkgs.buildWasmBindgenCli rec {
|
||||
src = pkgs.fetchCrate {
|
||||
pname = "wasm-bindgen-cli";
|
||||
@@ -73,14 +73,13 @@
|
||||
# `web/build.sh` — and so the Pages job's local dry run — needs this.
|
||||
# The `wasm` CI job only runs `cargo check`, which does not, so the
|
||||
# shell was missing the one tool that turns a passing check into a
|
||||
# loadable page (improvise-d31). Must match WASM_PACK_VERSION in
|
||||
# scripts/web-toolchain-versions.env; CI checks that it does.
|
||||
# loadable page (improvise-d31). CI takes whatever version this is.
|
||||
pkgs.wasm-pack
|
||||
wasmBindgenCli
|
||||
pkgs.binaryen
|
||||
|
||||
# NODE_VERSION's major, same check. The js and browser jobs ran 22
|
||||
# while `build` ran the pinned 24; one shell means one node.
|
||||
# The js and browser jobs once ran 22 while `build` ran 24; one shell
|
||||
# means one node.
|
||||
pkgs.nodejs_24
|
||||
pkgs.python3
|
||||
pkgs.jq
|
||||
|
||||
@@ -67,21 +67,7 @@ direct=$?
|
||||
unset name
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. The bash-5 `local` semantics. A multi-assignment `local` does not read
|
||||
# left to right on bash 5, so this suite passes on macOS bash 3.2 and dies
|
||||
# under `set -u` on bash 5 -- PR #133's 19/19 local against 2/19 in CI.
|
||||
d="$(sandbox bash5)"
|
||||
suite "$d" vers <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
f() { local a="$1" b="$a/x"; echo "$b"; }
|
||||
f one
|
||||
EOF
|
||||
expect "a bash-3.2-only construct fails under the runner's pinned bash" \
|
||||
1 "$d/checkers.sh" vers
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. The shebang rule. `#!/bin/sh` is dash in CI and bash-in-sh-mode on macOS.
|
||||
# 2. The shebang rule. `#!/bin/sh` is dash in CI and bash-in-sh-mode on macOS.
|
||||
d="$(sandbox shebang)"
|
||||
suite "$d" fine <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
@@ -101,7 +87,7 @@ rm -f "$d/legacy.sh"
|
||||
expect "and the same sandbox passes once it is removed" 0 "$d/checkers.sh" fine
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. A failing suite is reported, and a passing one is not.
|
||||
# 3. A failing suite is reported, and a passing one is not.
|
||||
d="$(sandbox rc)"
|
||||
suite "$d" red <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
@@ -117,52 +103,7 @@ EOF
|
||||
expect "a passing suite passes the runner" 0 "$d/checkers.sh" blue
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 5. No bash >= 5 and no nix is exit 3, not a silent run under whatever is
|
||||
# there. This is the arm that would have refused the macOS system bash.
|
||||
d="$(sandbox oldbash)"
|
||||
suite "$d" any <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
exit 0
|
||||
EOF
|
||||
mkdir -p "$work/fakebin"
|
||||
cat > "$work/fakebin/bash" <<'EOF'
|
||||
#!/bin/sh
|
||||
case "$*" in *BASH_VERSINFO*) echo 3; exit 0 ;; esac
|
||||
exit 0
|
||||
EOF
|
||||
chmod +x "$work/fakebin/bash"
|
||||
# The runner is invoked through a REAL bash, so the fake one below is only
|
||||
# what its own `command -v bash` probe finds -- not its interpreter.
|
||||
# utilbin carries only the utilities the runner itself needs, and no nix, so
|
||||
# the fake bash is the only candidate and the nix fallback is absent. Linked
|
||||
# one by one rather than taking /usr/bin:/bin, which on NixOS hold only `env`
|
||||
# and `sh` -- the runner died on `dirname` there, exit 1 instead of 3. `head`
|
||||
# and `env` are for 5b's 5.1 control, which runs past the version check.
|
||||
mkdir -p "$work/utilbin"
|
||||
for tool in dirname head env; do
|
||||
ln -sf "$(command -v "$tool")" "$work/utilbin/$tool"
|
||||
done
|
||||
env PATH="$work/fakebin:$work/utilbin" HOME="$HOME" \
|
||||
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 3 ] && ok "an old bash with no nix is exit 3" \
|
||||
|| bad "an old bash with no nix is exit 3" "exit $got, wanted 3"
|
||||
|
||||
# 5b. The floor is 5.1, not 5: `wait -n -p` is 5.1 (improvise-fd0j). The 5.1
|
||||
# control proves the 5.0 refusal is the version and not the fake itself.
|
||||
for v in "5 0:3:a bash 5.0 with no nix is exit 3, not 5.x" \
|
||||
"5 1:0:a bash 5.1 is accepted"; do
|
||||
printf '#!/bin/sh\ncase "$*" in *BASH_VERSINFO*) echo %s; exit 0 ;; esac\nexit 0\n' \
|
||||
"${v%%:*}" > "$work/fakebin/bash"
|
||||
want="${v#*:}"; name="${want#*:}"; want="${want%%:*}"
|
||||
env PATH="$work/fakebin:$work/utilbin" HOME="$HOME" \
|
||||
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = "$want" ] && ok "$name" || bad "$name" "exit $got, wanted $want"
|
||||
done
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 6. A named suite that does not exist is a usage error, not a silent no-op.
|
||||
# 4. A named suite that does not exist is a usage error, not a silent no-op.
|
||||
d="$(sandbox missing)"
|
||||
suite "$d" present <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
@@ -171,7 +112,7 @@ EOF
|
||||
expect "an unknown suite name is exit 4" 4 "$d/checkers.sh" absent
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 7. --list names every suite in the directory and nothing else.
|
||||
# 5. --list names every suite in the directory and nothing else.
|
||||
d="$(sandbox listing)"
|
||||
suite "$d" alpha <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
|
||||
+10
-39
@@ -3,15 +3,14 @@
|
||||
# and so does the local gate, so a suite cannot be green under one shell and
|
||||
# red under the other. Reasoning: context/code/tooling.md section 3.
|
||||
#
|
||||
# It does three things a bare `./scripts/X-test.sh` does not:
|
||||
# It does two things a bare `./scripts/X-test.sh` does not. It runs every
|
||||
# suite under the bash running it (the dev shell's), and it:
|
||||
#
|
||||
# 1. pins a bash >= 5.1, taken from nix when the ambient one is older, so the
|
||||
# macOS system bash 3.2 can never be the shell a suite passes under;
|
||||
# 2. scrubs the environment down to an allowlist, because `nix develop`
|
||||
# 1. scrubs the environment down to an allowlist, because `nix develop`
|
||||
# exports `name`, `out`, `system`, `shell`, `builder` and two dozen more
|
||||
# generically-named variables that CI does not have -- a `set -u` fault
|
||||
# reading one of them is SILENT inside the dev shell and fatal in CI;
|
||||
# 3. refuses a shell script in scripts/ whose shebang is not
|
||||
# 2. refuses a shell script in scripts/ whose shebang is not
|
||||
# `#!/usr/bin/env bash`, so the sh/bash split cannot come back.
|
||||
set -euo pipefail
|
||||
|
||||
@@ -30,7 +29,6 @@ usage: checkers.sh [--list] [--self-test] [name ...]
|
||||
|
||||
exit 0 every suite passed
|
||||
exit 1 a suite failed, or a shebang is not `#!/usr/bin/env bash`
|
||||
exit 3 no bash >= 5.1 could be found and nix could not supply one
|
||||
exit 4 usage error
|
||||
|
||||
THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same
|
||||
@@ -46,22 +44,6 @@ ALLOW="HOME TMPDIR TMP TEMP TERM LANG LC_ALL USER LOGNAME
|
||||
NIX_PATH NIX_REMOTE NIX_SSL_CERT_FILE SSL_CERT_FILE
|
||||
XDG_CACHE_HOME XDG_CONFIG_HOME XDG_DATA_HOME"
|
||||
|
||||
# Prints the absolute path of a bash >= 5.1, or nothing. The ambient one is tried
|
||||
# first so a machine that already has a modern bash pays no nix startup at all.
|
||||
find_modern_bash() {
|
||||
local candidate major minor
|
||||
candidate="$(command -v bash || true)"
|
||||
if [ -n "$candidate" ]; then
|
||||
read -r major minor < <("$candidate" -c 'echo "${BASH_VERSINFO[0]} ${BASH_VERSINFO[1]}"' \
|
||||
2>/dev/null || echo 0 0)
|
||||
if [ "$(( ${major:-0} * 100 + ${minor:-0} ))" -ge 501 ] 2>/dev/null; then
|
||||
echo "$candidate"; return 0
|
||||
fi
|
||||
fi
|
||||
command -v nix >/dev/null 2>&1 || return 0
|
||||
nix shell nixpkgs#bash --command bash -c 'command -v bash' 2>/dev/null || true
|
||||
}
|
||||
|
||||
# Every shell script in scripts/ says `#!/usr/bin/env bash` and nothing else.
|
||||
# `#!/bin/sh` is dash in CI and bash 3.2 in sh-mode on macOS: a second platform
|
||||
# split, undetected for the same reason the first one was.
|
||||
@@ -94,19 +76,16 @@ suites() {
|
||||
done
|
||||
}
|
||||
|
||||
# $1 the pinned bash, $2 the suite file. The suite runs with the pinned bash
|
||||
# FIRST on PATH, so a script it invokes through its own `#!/usr/bin/env bash`
|
||||
# -- or through `sh` -- gets the pinned one too, not the system 3.2.
|
||||
# $1 the suite file, run by the bash running this script, with PATH unchanged.
|
||||
run_suite() {
|
||||
local pinned="$1" file="$2"
|
||||
local bindir; bindir="$(dirname "$pinned")"
|
||||
local file="$1"
|
||||
local -a env_args=(-i)
|
||||
local v
|
||||
for v in $ALLOW ${CHECKERS_KEEP:-}; do
|
||||
if [ -n "${!v+set}" ]; then env_args+=("$v=${!v}"); fi
|
||||
done
|
||||
env_args+=("PATH=$bindir:${PATH:-/usr/bin:/bin}")
|
||||
env "${env_args[@]}" "$pinned" "$file"
|
||||
env_args+=("PATH=$PATH")
|
||||
env "${env_args[@]}" "$BASH" "$file"
|
||||
}
|
||||
|
||||
main() {
|
||||
@@ -123,15 +102,7 @@ main() {
|
||||
shift
|
||||
done
|
||||
|
||||
local pinned; pinned="$(find_modern_bash)"
|
||||
if [ -z "$pinned" ]; then
|
||||
echo 'checkers: no bash >= 5.1 on PATH and nix cannot supply one.' >&2
|
||||
echo ' Enter the dev shell (direnv allow / nix develop) and retry.' >&2
|
||||
return 3
|
||||
fi
|
||||
printf 'checkers: bash %s at %s\n' \
|
||||
"$("$pinned" -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"')" "$pinned"
|
||||
|
||||
printf 'checkers: bash %s at %s\n' "$BASH_VERSION" "$BASH"
|
||||
local rc=0
|
||||
check_shebangs || rc=1
|
||||
|
||||
@@ -149,7 +120,7 @@ main() {
|
||||
rc=4; continue
|
||||
fi
|
||||
printf '\n=== %s ===\n' "$n"
|
||||
run_suite "$pinned" "$file" || rc=1
|
||||
run_suite "$file" || rc=1
|
||||
done
|
||||
return $rc
|
||||
}
|
||||
|
||||
+1
-1
@@ -87,7 +87,7 @@ ledger_path() {
|
||||
# first line; bytes alone are not enough, since a restore can reproduce them.
|
||||
file_stamp() {
|
||||
printf '%s %s' "$(stamp_of "$1")" \
|
||||
"$(stat -c '%i %.9Y' "$1" 2>/dev/null || stat -f '%i %Fm' "$1")"
|
||||
"$(stat -c '%i %.9Y' "$1")"
|
||||
}
|
||||
ledger_holds() { [ -f "$1" ] && [ "$(head -n 1 "$1")" = "% kb-stamp $(file_stamp "$2")" ]; }
|
||||
|
||||
|
||||
@@ -9,12 +9,13 @@ work="$(mktemp -d)"
|
||||
trap 'rm -rf "$work"' EXIT
|
||||
|
||||
# Every run is bounded, so a loop that never stops FAILS (exit 124) rather than
|
||||
# hanging the suite. /bin/sleep, because some cases put a no-op `sleep` first on PATH.
|
||||
# hanging the suite. The real sleep, resolved before any case stubs PATH; NixOS has no /bin/sleep.
|
||||
BOUND=20
|
||||
REAL_SLEEP="$(type -P sleep)"
|
||||
bounded_reviews() {
|
||||
"$here/pr-reviews" "$@" &
|
||||
local pid=$! dog done rc
|
||||
/bin/sleep "$BOUND" &
|
||||
"$REAL_SLEEP" "$BOUND" &
|
||||
dog=$!
|
||||
wait -n -p done "$pid" "$dog"; rc=$?
|
||||
if [ "$done" = "$pid" ]; then kill "$dog" 2>/dev/null; wait "$dog" 2>/dev/null; return "$rc"; fi
|
||||
@@ -218,7 +219,7 @@ clock="$work/clock"
|
||||
mkdir -p "$clock"
|
||||
cat > "$clock/date" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
[ "${1:-}" = "+%s" ] || exec /bin/date "$@"
|
||||
[ "$*" = "+%s" ] || { echo "fake date: only +%s is faked, got: $*" >&2; exit 2; }
|
||||
n=$(( $(cat "$CLOCK_TICKS" 2>/dev/null || echo 0) + 1 ))
|
||||
echo "$n" > "$CLOCK_TICKS"
|
||||
echo $(( 1000 + n ))
|
||||
|
||||
Reference in New Issue
Block a user