Split ghostel-test.el into per-topic files#290
Merged
Merged
Conversation
The single 16,147-line ghostel-test.el is now 16 ghostel-*-test.el files plus a shared ghostel-test-helpers.el. Tests that need the Zig native module carry :tags '(native); the runner functions select via (tag native) / (not (tag native)) instead of the hand-maintained ghostel-test--elisp-tests whitelist. Functionally equivalent to before: same 589 tests, same pass/fail outcomes (whitelist had one stale entry, ghostel-test-eshell, with no matching deftest — so effective elisp count is 398, native 191). Per-file sizes range from 93 lines (exec) to 2110 lines (scrollback). Section-local helpers (with-cat-process, kitty-fixture, with-input-fixture, etc.) move with their tests; cross-bucket helpers (with-compile-buffer, row0, cursor, wait-for) live in ghostel-test-helpers.el. File-local dynamic-var declarations preserved per-file: mouse-paste-test.el needs (defvar xterm-store-paste-on-kill-ring) and scrollback-test.el needs the preedit-overlay defvars — without these the cross-file let bindings would be lexical and the wrapped code wouldn't see the rebound value.
A review pass over the per-topic test files surfaced ~30 places where tests were passing for the wrong reasons: tautological assertions, mocks of the function under test, or — in two cases — references to symbols that no longer exist. None of these are regressions from the split; they were inherited from the monolithic file and became easier to spot once tests were organised by topic. Highlights: - exec / tramp: the size-setter mock was `ghostel--set-size` but the real code path now calls `ghostel--set-size-with-cell-dims`. The mock never fired. Renamed the symbol; exec also now asserts all 5 arguments to `ghostel--new` instead of the first 2. - keys: `ghostel-test-special-key-modifier-bindings` was querying `ghostel-mode-map`, where no special-key bindings exist. The `(when binding ...)` guard hid that the test covered zero keys. Fix points it at `ghostel-semi-char-mode-map` and handles the documented `S-<insert>` -> `ghostel-yank' exception. Same file also tightens `(should binding)' to `(should (commandp binding))' so a stray prefix-arg or sub-keymap can't pass for "command." - terminal: `ignore-cursor-change' now seeds `cursor-type' to a known-different value before each call, so the assertion proves the function mutated state instead of matching a coincident default. - line-mode: `cursor-point-tracks-cursor-char-pos' compared the variable to a trivial accessor returning it. Replaced with a point-moved check (cursor follows char-pos, not point) and the documented nil-when-no-cursor branch. - modes: dropped tautological `(should global-hl-line-mode)' that re-asserted a `let' binding; added missing `terminal-frozen-p' predicate in the char-mode block (semi-char and copy already had it); rewrote `copy-mode-recenter' to verify the window actually recenters instead of mocking out `recenter'. - project: `project-universal-arg' now captures `ghostel-buffer-name' at `ghostel' call time so the test proves the project-prefixed binding (`*myproj-ghostel*') actually took effect, instead of only observing the prefix-arg passthrough. - osc: switched mocks from the dispatchers (`ghostel--osc-progress', `ghostel--handle-notification') to the sinks (`ghostel-progress-function', `ghostel-notification-function') so the real dispatch + string->symbol conversion runs; expected values updated to match. `default-notify' now also asserts the wiring via `ghostel--handle-notification'. - render: `apply-palette-ghostel-default-face' now captures the arguments to `ghostel--set-default-colors' to prove the function consumed the mocked colour value rather than a hardcoded one; `coalesces-plain-link-detection' moves its assertion out of the mock body. - glyph-kitty: `glyph-adjust-covered-by-main-font' had a vacuous "font-at would be unbound" guard (font-at is always bound). Replaced with a real `cl-letf' trip-wire over `font-at' that fails if called; added a `;; FIXME:' on `kitty-graphics-emit-end-to-end' whose mock obliterates the function under test. Added a null-check before `(cadr (assq ...))' so a missing `min-width' entry fails clearly instead of via the confusing `(equal nil '(2))' form. - scrollback / mouse-paste: minimised stubs and added `;; FIXME:' comments on the few tests whose end-to-end rewrite needs a real terminal/window fixture. Test results unchanged: 78 zig + 191 native + 415 elisp pass; only the two pre-existing `bash-completion' skips.
Each ghostel-*-test.el now gets its own .build/tests/<kind>-<name>.ok stamp. `test' and `test-native' depend on the full set, so `make -j$(nproc)' fan-outs across cores — the slowest single file sets the wall-clock floor, not the sum. Numbers on this machine (warm caches): - make -j8 test: 12.5s -> 6.6s - make -j8 test-native: 11.2s -> 7.8s - make -j8 all: 10.4s -> 8.4s - make -j8 all (re-run, no changes): ~0.9s The slowest two files are tramp-test (~5.5s, SIGWINCH subprocesses) and shell-test (~5.2s, fish/zsh subprocess spawning). Further wins would require splitting those, which is out of scope here. Stamps invalidate on the right inputs: - elisp stamps depend on the test file, ghostel-test-helpers.el, and $(ELC). Editing one test file re-runs only that one. - native stamps additionally depend on $(MODULE), and $(MODULE) is a real-file target depending on $(ZIG_SOURCES). Zig is invoked only when sources actually change, not on every `make' invocation. Recommended invocation now documented in the Makefile: make -j$(nproc) all # Linux make -j$(sysctl -n hw.ncpu) all # macOS make -j$(getconf _NPROCESSORS_ONLN) all # portable New EMACSFLAGS variable lets callers inject load paths into every emacs invocation (CI uses it for `-L /tmp/compat'). CI updated to use the new targets: - byte-compile, test, test-native steps now call `make -jN' with EMACSFLAGS=-L /tmp/compat. test-evil left as a direct emacs invocation since its runner is the older whitelist-based file. - test-native touches the downloaded module artifact so its mtime is newer than the Zig sources (otherwise make would try to re-invoke `zig build', which isn't installed in the test-native job). - `-O target' is not passed because macOS ships GNU make 3.81. `.build/' added to .gitignore; `clean' removes it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test/ghostel-test.elis now 16ghostel-*-test.elfiles plus a sharedtest/ghostel-test-helpers.el. Per-file sizes range from 93 lines (exec) to 2,110 lines (scrollback).:tags '(native). The runner functions select via(tag native)/(not (tag native))instead of the hand-maintainedghostel-test--elisp-testswhitelist (gone).Makefileloads alltest/ghostel-*-test.elvia wildcard, so adding a new file requires no Makefile edit.checkdocalso picks them up viafile-expand-wildcards.Why
ghostel-test.elwas the largest file in the repo. New tests landed in whichever section vaguely matched, helpers piled up at the top, and the whitelist had to be updated by hand every time a new elisp-only test arrived (and silently rotted when a name changed). The split organizes by topic so new tests have an obvious home, and the tag-based runner makes the elisp/native split a property of each test rather than a separately-maintained list.What's in each file
ghostel-scrollback-test.elghostel-shell-test.elghostel-line-mode-test.elghostel-compile-test.elghostel-compile--finalize, recompile, global/toggle mode, interactive form, mode-lineghostel-tramp-test.elghostel-mouse-paste-test.elghostel-render-test.elghostel-osc-test.elghostel-glyph-kitty-test.elghostel-module-test.elghostel-modes-test.elghostel-keys-test.elghostel-debug-test.elghostel-debug-keypress,ghostel-debug-infosectionsghostel-project-test.elghostel-projectbuffer naming, identity match, return-buffer semanticsghostel-terminal-test.elghostel-exec-test.elghostel-execandghostel-eshellintegrationHelpers used by >1 bucket (
with-compile-buffer,row0,cursor,wait-for) live inghostel-test-helpers.el. Section-local helpers (with-cat-process,kitty-fixture,with-input-fixture,with-spawn-capture,with-focus-stub, the glyph-mock family, etc.) move with their tests.File-local
(defvar X)declarations are preserved per-file where needed (mouse-paste-test.elforxterm-store-paste-on-kill-ring,scrollback-test.elfor the preedit-overlay vars). Without them, cross-fileletbindings would be lexical and the wrapped code wouldn't see the rebound value — caught by an initial test failure during this work.Functional equivalence
Same 589 tests, same pass/fail outcomes. The original whitelist had 399 entries but one (
ghostel-test-eshell) was stale — no matching deftest existed. Effective elisp count is 398 + 191 native = 589.ghostel-test-fish-backspacealready carried:tags '(:fish)and now carries:tags '(:fish native).Known pre-existing test quality issues (not addressed here)
A review pass over the split files surfaced some tests that pass but don't actually exercise their target. None of these are introduced by this PR — they already existed in the monolith. Filing separately as follow-up; highest-priority items:
test/ghostel-exec-test.el:57andtest/ghostel-tramp-test.el:940both mockghostel--set-size, but the real code path now callsghostel--set-size-with-cell-dims. Mocks never fire; tests pass vacuously.modes-test.el:117,line-mode-test.el:272,mouse-paste-test.el:302/315,tramp-test.el:1117).mouse-paste-test.elmocksghostel-readonly-exitin 8 readonly tests, so the actual exit logic is never end-to-end tested.Test plan
make -j4 allclean: 78 zig + 191 native + 415 elisp pass, 0 unexpected, 2 pre-existing skips (bash-completion).make test-evilclean: 78/78.ghostel-test-*deftests in old vs. new layout, no duplicates across files.:tags '(native)recognized and docstrings preserved (docstring must come before:tagsfor ert to keep it).