diff --git a/doc/agents/herdr.md b/doc/agents/herdr.md index ff214bd..c247bd6 100644 --- a/doc/agents/herdr.md +++ b/doc/agents/herdr.md @@ -7,9 +7,9 @@ JSON it reads, and what each one can surprise you with. ## Calls -Each call runs `herdr` from PATH through `vim.system` with a 5-second timeout. A spawn that -fails, the binary gone since the check or an argv over the OS limit, counts as a refused -call. +Each call runs `herdr` from PATH through `Process.run` with a 5-second timeout. A spawn +that fails, the binary gone since the check or an argv over the OS limit, counts as a +refused call. | Command | Reads | | -------------------------------------- | --------------------------------------------------------- | diff --git a/doc/changeset.nvim.txt b/doc/changeset.nvim.txt index 9f34c0c..e02457a 100644 --- a/doc/changeset.nvim.txt +++ b/doc/changeset.nvim.txt @@ -173,7 +173,8 @@ and changeset warns "no merge base with the default branch". Reopened on a tree kept from before, the sidebar shows it at once, then warns and closes once it measures the base again and finds none. A commit or rebase that leaves none keeps the tree as it was. Changeset measures the base without holding up -the editor, and the sidebar opens once it has. +the editor, and the sidebar opens once it has. An error inside changeset while +it measures the base is reported, then treated as finding no base. ============================================================================== 5. Commands *changeset-commands* diff --git a/lua/changeset/diff.lua b/lua/changeset/diff.lua index a4b97e3..b9edf20 100644 --- a/lua/changeset/diff.lua +++ b/lua/changeset/diff.lua @@ -1,6 +1,6 @@ ---Runs the branch's git diff and parses it into files and hunks, and reads a file's text at the base. -local Git = require("changeset.git") +local Process = require("changeset.process") local sections = require("changeset.sections") local M = {} @@ -300,11 +300,11 @@ end ---Run every command concurrently and hand all results to `on_done` on the main loop. ---@param commands table ---@param cwd string ----@param on_done fun(results: table) +---@param on_done fun(results: table) local function run_all(commands, cwd, on_done) local results, pending = {}, vim.tbl_count(commands) for name, argv in pairs(commands) do - Git.system(argv, { cwd = cwd, text = true }, function(result) + Process.run(argv, { cwd = cwd }, function(result) results[name] = result pending = pending - 1 if pending == 0 then @@ -314,20 +314,20 @@ local function run_all(commands, cwd, on_done) end end ----@param results table +---@param results table ---@return string? stderr Of a failed command, if any. local function first_failure(results) for _, result in pairs(results) do if result.code ~= 0 then - return vim.trim(result.stderr or "") + return vim.trim(result.stderr) end end end ----@param result vim.SystemCompleted +---@param result changeset.process.Completed ---@return string[] local function stdout_lines(result) - return vim.split(result.stdout or "", "\n", { trimempty = true }) + return vim.split(result.stdout, "\n", { trimempty = true }) end ---Line count and content digest per readable file; directories (untracked nested repos) and dangling symlinks are skipped. @@ -372,12 +372,12 @@ local function generated_paths(files, cwd, on_done) local paths = vim.tbl_map(function(file) return file.path end, files) - Git.system( + Process.run( { "git", "check-attr", "-z", "--stdin", "linguist-generated" }, - { cwd = cwd, text = true, stdin = table.concat(paths, "\0") }, + { cwd = cwd, stdin = table.concat(paths, "\0") }, function(result) -- A failed read only costs files their Generated section. - local marked = result.code == 0 and M._parse_check_attr(result.stdout or "") or {} + local marked = result.code == 0 and M._parse_check_attr(result.stdout) or {} for _, file in ipairs(files) do if file.status ~= "deleted" @@ -406,10 +406,10 @@ function M.collect(base, cwd, callback) end local hunks, digests = M._parse_hunks(stdout_lines(results.hunks)) local untracked, untracked_digests = - count_lines(vim.split(results.untracked.stdout or "", "\0", { trimempty = true }), cwd) + count_lines(vim.split(results.untracked.stdout, "\0", { trimempty = true }), cwd) local files = M._assemble({ - numstat = M._parse_numstat(results.numstat.stdout or ""), - statuses = M._parse_name_status(results.name_status.stdout or ""), + numstat = M._parse_numstat(results.numstat.stdout), + statuses = M._parse_name_status(results.name_status.stdout), hunks = hunks, untracked = untracked, -- A path is both deleted and untracked after `git rm --cached`; the untracked digest wins so both rows share @@ -431,7 +431,7 @@ end ---@param cwd string ---@param callback fun(text: string?) function M.blob(object, cwd, callback) - Git.system({ "git", "cat-file", "blob", object }, { cwd = cwd, text = true }, function(result) + Process.run({ "git", "cat-file", "blob", object }, { cwd = cwd }, function(result) callback(result.code == 0 and result.stdout or nil) end) end diff --git a/lua/changeset/fork_point.lua b/lua/changeset/fork_point.lua index fb370eb..d748bbd 100644 --- a/lua/changeset/fork_point.lua +++ b/lua/changeset/fork_point.lua @@ -242,8 +242,9 @@ local function measure_async(root, branch, settle, on_done) end ---The fork point of `branch` at `root`, measured without blocking Neovim and handed to `on_done` on the main loop, nil ----when HEAD shares none with the branch it was created from or the default branch. Asks gh about its PR unless gh has ----named a target or is being asked. Subscribers hear the point after `on_done` does. +---when HEAD shares none with the branch it was created from or the default branch, or when measuring it raised an +---error, which is reported. Asks gh about its PR unless gh has named a target or is being asked. Subscribers hear the +---point after `on_done` does. ---@param root string ---@param branch string ---@param on_done fun(point: changeset.ForkPoint?) diff --git a/lua/changeset/git.lua b/lua/changeset/git.lua index 5e95d88..9dfafaa 100644 --- a/lua/changeset/git.lua +++ b/lua/changeset/git.lua @@ -1,22 +1,7 @@ ----The git and gh queries that pick the branch changeset diffs against, HEAD read off git's own files, and the process ----runner they and herdr share. -local M = {} +---The git and gh queries that pick the branch changeset diffs against, and HEAD read off git's own files. +local process = require("changeset.process") ----`vim.system`, calling `on_exit` on the main loop. A failed spawn — a repository removed under a pending ----refresh, a binary gone since its executable check, an argv over the OS's limit — reaches `on_exit` as a failed ----result instead of raising. ----@param argv string[] ----@param opts vim.SystemOpts ----@param on_exit fun(result: vim.SystemCompleted) -function M.system(argv, opts, on_exit) - local ok, err = pcall(vim.system, argv, opts, vim.schedule_wrap(on_exit)) - if not ok then - local result = { code = -1, signal = 0, stdout = "", stderr = tostring(err) } - vim.schedule(function() - on_exit(result) - end) - end -end +local M = {} ---The first line of the file at `path`; nil when it can't be read, as a directory can't. ---@param path string @@ -94,31 +79,33 @@ local async_threads = setmetatable({}, { __mode = "k" }) ---In a coroutine `async` runs, `args`' result, waited for without blocking Neovim; nil elsewhere. ---@param args string[] ----@return vim.SystemCompleted? +---@return changeset.process.Completed? local function await(args) local co = coroutine.running() if co and async_threads[co] then - return (coroutine.yield --[[@as fun(args: string[]): vim.SystemCompleted]])(args) + return (coroutine.yield --[[@as fun(args: string[]): changeset.process.Completed]])(args) end end ---Run `fn` with every git command `lines` and `is_ancestor` start inside it spawned without blocking Neovim, then ----call `on_done` on the main loop with what `fn` returns. +---call `on_done` on the main loop with what `fn` returns, or with nil after reporting the error `fn` raised. Start it +---on the main loop, as `process.run` needs. ---@generic T ---@param fn fun(): T ----@param on_done fun(result: T) +---@param on_done fun(result: T?) function M.async(fn, on_done) local co = coroutine.create(fn) async_threads[co] = true local function step(...) local ok, value = coroutine.resume(co, ...) if not ok then - error(debug.traceback(co, value)) + vim.notify("Changeset: " .. debug.traceback(co, tostring(value)), vim.log.levels.ERROR) + return on_done(nil) end if coroutine.status(co) == "dead" then return on_done(value) end - M.system(value, { text = true }, step) + process.run(value, {}, step) end step() end @@ -137,7 +124,7 @@ function M.lines(args, cwd) if result.code ~= 0 then return {} end - local out = vim.split(result.stdout or "", "\n", { plain = true }) + local out = vim.split(result.stdout, "\n", { plain = true }) if out[#out] == "" then out[#out] = nil end @@ -287,16 +274,12 @@ function M.pr_target(cwd, cb) cb(nil, nil, true) end) end - M.system( - { "gh", "pr", "view", "--json", "baseRefName,number,state" }, - { cwd = cwd, text = true, timeout = 5000 }, - function(res) - local ok, pr = pcall(vim.json.decode, res.stdout or "") - local answered = res.code == 0 and ok and type(pr) == "table" - local open = answered and pr.state == "OPEN" - cb(open and pr.baseRefName or nil, open and pr.number or nil, not answered) - end - ) + process.run({ "gh", "pr", "view", "--json", "baseRefName,number,state" }, { cwd = cwd, timeout = 5000 }, function(res) + local ok, pr = pcall(vim.json.decode, res.stdout) + local answered = res.code == 0 and ok and type(pr) == "table" + local open = answered and pr.state == "OPEN" + cb(open and pr.baseRefName or nil, open and pr.number or nil, not answered) + end) end return M diff --git a/lua/changeset/herdr.lua b/lua/changeset/herdr.lua index dfe49e3..f19b98b 100644 --- a/lua/changeset/herdr.lua +++ b/lua/changeset/herdr.lua @@ -3,6 +3,7 @@ local comment_store = require("changeset.comment_store") local dialog = require("changeset.dialog") local git = require("changeset.git") local highlights = require("changeset.highlights") +local process = require("changeset.process") local M = {} @@ -186,13 +187,13 @@ end ---@param args string[] ---@param cb fun(result: table?, code: string?) local function herdr(args, cb) - git.system(vim.list_extend({ "herdr" }, args), { text = true, timeout = TIMEOUT }, function(res) + process.run(vim.list_extend({ "herdr" }, args), { timeout = TIMEOUT }, function(res) if res.code ~= 0 then - local ok, env = pcall(vim.json.decode, res.stderr or "") + local ok, env = pcall(vim.json.decode, res.stderr) cb(nil, ok and type(env) == "table" and type(env.error) == "table" and env.error.code or "") return end - local ok, out = pcall(vim.json.decode, res.stdout or "", { luanil = { object = true, array = true } }) + local ok, out = pcall(vim.json.decode, res.stdout, { luanil = { object = true, array = true } }) cb(ok and type(out) == "table" and out.result or {}, nil) end) end diff --git a/lua/changeset/process.lua b/lua/changeset/process.lua new file mode 100644 index 0000000..c989dcf --- /dev/null +++ b/lua/changeset/process.lua @@ -0,0 +1,74 @@ +---Starts the processes changeset doesn't wait on, through `jobstart`, handing each one's exit code and output back on +---the main loop. +local M = {} + +---What `run` hands `argv` and how long it lets it run. +---@class changeset.process.Opts +---@field cwd string? +---@field stdin string? Written to the process, then closed; without it, the process's stdin is null. +---@field timeout integer? Milliseconds. + +---What a command `run` started exited with and printed. +---@class changeset.process.Completed +---@field code integer -1 when it could not start; 124 when it ran out of time. +---@field stdout string +---@field stderr string What it printed to stderr; the reason when it could not start. + +---A buffered `jobstart` stream's bytes, taking over `data`: its list carries each NUL as "\n" inside an item. CRLF +---reads as LF, so a CRLF file's base text shows and parses without a "\r" on every line. +---@param data string[] +---@return string +local function joined(data) + for i, item in ipairs(data) do + if item:find("\n", 1, true) then + data[i] = item:gsub("\n", "\0") + end + end + local text = table.concat(data, "\n") + return text:find("\r\n", 1, true) and (text:gsub("\r\n", "\n")) or text +end + +---Start `argv` through `jobstart`, whose spawn blocks Neovim a fraction as long as `vim.system`'s, calling +---`on_result` on the main loop. Start it on the main loop: in a fast event `jobstart` can't spawn, and the command +---reads as failed. A failed spawn — a repository removed under a pending refresh, a binary gone since its executable +---check, an argv over the OS's limit — reaches `on_result` as a failed result instead of raising. +---@param argv string[] +---@param opts changeset.process.Opts +---@param on_result fun(result: changeset.process.Completed) +function M.run(argv, opts, on_result) + local stdout, stderr = {}, {} ---@type string[], string[] + local timed_out = false + local ok, job = pcall(vim.fn.jobstart, argv, { + cwd = opts.cwd, + stdin = opts.stdin and "pipe" or "null", + stdout_buffered = true, + stderr_buffered = true, + on_stdout = function(_, data) + stdout = data + end, + on_stderr = function(_, data) + stderr = data + end, + on_exit = function(_, code) + on_result({ code = timed_out and 124 or code, stdout = joined(stdout), stderr = joined(stderr) }) + end, + }) + if not ok or job <= 0 then + local reason = ok and ("could not start " .. argv[1]) or tostring(job) + vim.schedule(function() + on_result({ code = -1, stdout = "", stderr = reason }) + end) + return + end + if opts.stdin then + vim.fn.chansend(job, opts.stdin) + vim.fn.chanclose(job, "stdin") + end + if opts.timeout then + vim.defer_fn(function() + timed_out = vim.fn.jobstop(job) == 1 + end, opts.timeout) + end +end + +return M diff --git a/tests/build_spec.lua b/tests/build_spec.lua index 55e9991..c3aef78 100644 --- a/tests/build_spec.lua +++ b/tests/build_spec.lua @@ -229,22 +229,28 @@ describe("changeset.build", function() end) describe("while gh is slow to answer", function() - before_each(function() - vim.env.FAKE_GH_DELAY = "1" - end) + local held ---@type support.spawns.Held? after_each(function() - vim.env.FAKE_GH_DELAY = nil + if held then + held.stop() + held = nil + end end) it("asks gh once when HEAD's fork point moves before it answers no PR", function() local argvs = recording(function() + -- Held until HEAD's fork point has moved, however long moving it takes under load. + held = spawns.holding(function(argv) + return not runs(argv, "gh") + end) build_and_collect() Fixture.git({ "checkout", "-q", "trunk" }, tmp) Fixture.git({ "commit", "-q", "--allow-empty", "-m", "trunk work" }, tmp) Fixture.git({ "checkout", "-q", "feature" }, tmp) Fixture.git({ "rebase", "-q", "trunk" }, tmp) build_and_collect() + present(held).release() -- Long enough for the first answer to land and anything it starts to ask gh again. vim.wait(3000, function() return false @@ -601,11 +607,17 @@ describe("changeset.build", function() end) describe("once gh names a PR onto the branch already compared against", function() + local held ---@type support.spawns.Held? + before_each(function() vim.env.FAKE_GH_PR = gh.pr_view({ baseRefName = "trunk", number = 7 }) end) after_each(function() + if held then + held.stop() + held = nil + end vim.env.FAKE_GH_PR = nil end) @@ -620,18 +632,21 @@ describe("changeset.build", function() end) it("hears the PR while the current buffer is outside the tree's repository", function() - vim.env.FAKE_GH_DELAY = "1" local elsewhere = vim.fn.tempname() vim.fn.mkdir(elsewhere, "p") + -- Held until the current buffer has left the tree's repository. + held = spawns.holding(function(argv) + return not runs(argv, "gh") + end) build_and_collect() vim.fn.chdir(elsewhere) vim.cmd("enew") vim.bo.buftype = "nofile" + present(held).release() local heard = vim.wait(5000, function() return vim.list_contains(events, "pr") end, 25) - vim.env.FAKE_GH_DELAY = nil vim.fn.delete(elsewhere, "rf") assert.is_true(heard) assert.equal(7, present(build.current()).pr) @@ -688,6 +703,7 @@ describe("changeset.build on a branch measured against its PR's target", functio local previous_dir ---@type string local source ---@type support.symbols.Source local parent_base ---@type string + local held ---@type support.spawns.Held? ---@param name string ---@return string @@ -724,8 +740,11 @@ describe("changeset.build on a branch measured against its PR's target", functio end) after_each(function() + if held then + held.stop() + held = nil + end vim.env.FAKE_GH_PR = nil - vim.env.FAKE_GH_DELAY = nil source.restore() vim.cmd("silent! %bwipeout!") vim.fn.chdir(previous_dir) @@ -735,11 +754,16 @@ describe("changeset.build on a branch measured against its PR's target", functio it("keeps the tree on the PR's target across a commit", function() build_on_pr() local kept = present(build.current()) - vim.env.FAKE_GH_DELAY = "0.5" - commit_file("more.txt") + -- Held, so the re-measure lands while gh is still being asked again. + held = spawns.holding(function(argv) + return not runs(argv, "gh") + end) + local head = commit_file("more.txt") build.update() - vim.wait(400) + assert.is_true(vim.wait(5000, function() + return (build.current() or {}).head == head + end, 10)) assert.equal(kept, build.current()) assert.equal(parent_base, kept.base) @@ -763,14 +787,22 @@ describe("changeset.build on a branch measured against its PR's target", functio it("measures against the PR's target at once when switching back to the branch", function() build_on_pr() + -- Held, so the switch back can measure only from the target gh named before. + held = spawns.holding(function(argv) + return not runs(argv, "gh") + end) Fixture.git({ "switch", "-q", "parent" }, root) build.update() - vim.env.FAKE_GH_DELAY = "0.5" + assert.is_true(vim.wait(5000, function() + return (build.current() or {}).branch == "parent" + end, 10)) Fixture.git({ "switch", "-q", "feature" }, root) build.update() + assert.is_true(vim.wait(5000, function() + return (build.current() or {}).branch == "feature" + end, 10)) - assert.equal("feature", present(build.current()).branch) assert.equal(parent_base, present(build.current()).base) end) diff --git a/tests/diff_spec.lua b/tests/diff_spec.lua index 09c5726..7165835 100644 --- a/tests/diff_spec.lua +++ b/tests/diff_spec.lua @@ -1,6 +1,8 @@ local diff = require("changeset.diff") local Fixture = require("support.git") local present = require("support.present") +local spawns = require("support.spawns") +local Process = require("changeset.process") -- Real `git diff --numstat -M -z ` output. A rename's two paths follow an empty path field; -- binary files report `-` for both counts. @@ -797,21 +799,21 @@ describe("changeset.diff.collect", function() it("calls back when the repository vanishes before the generated-file read", function() local base = seed_edited_file(tmp) - local system = vim.system + local real = Process.run local check_attr_ran = false - vim.system = function(argv, ...) - if vim.tbl_contains(argv, "check-attr") then + Process.run = function(argv, ...) + if spawns.runs(argv, "git", "check-attr") then check_attr_ran = true vim.fn.delete(tmp, "rf") end - return system(argv, ...) + return real(argv, ...) end local files ---@type changeset.File[]? local err local ok, raised = pcall(function() files, err = await_collect(base, tmp) end) - vim.system = system + Process.run = real ---@cast raised string? assert.is_true(ok, raised) assert.is_true(check_attr_ran) @@ -822,7 +824,7 @@ describe("changeset.diff.collect", function() end) it("reports a missing repository as an error", function() - assert.matches("ENOENT", present((select(2, await_collect("HEAD", tmp .. "/gone"))))) + assert.matches(".", present((select(2, await_collect("HEAD", tmp .. "/gone"))))) end) end) diff --git a/tests/fork_point_spec.lua b/tests/fork_point_spec.lua index 8427ddd..8be68ab 100644 --- a/tests/fork_point_spec.lua +++ b/tests/fork_point_spec.lua @@ -75,6 +75,7 @@ end describe("fork_point", function() local real_pr_target, asks local roots = {} + local held_gh ---@type support.spawns.Held? before_each(function() asks = 0 @@ -86,9 +87,12 @@ describe("fork_point", function() end) after_each(function() + if held_gh then + held_gh.stop() + held_gh = nil + end Git.pr_target = real_pr_target vim.env.FAKE_GH_PR = nil - vim.env.FAKE_GH_DELAY = nil for _, root in ipairs(roots) do vim.fn.delete(root, "rf") end @@ -310,7 +314,6 @@ describe("fork_point", function() local root = repo() get(root, "feature") assert.is_true(await_heard(root, 1)) - vim.env.FAKE_GH_DELAY = "0.3" fork_point.recheck(root, "feature") fork_point.recheck(root, "feature") @@ -329,26 +332,18 @@ describe("fork_point", function() measures = measures + 1 return real_get_async(...) end - local real_system, held = vim.system, {} - vim.system = function(argv, opts, on_exit) - if argv[1] == "gh" then - return real_system(argv, opts, on_exit) - end - held[#held + 1] = { argv, opts, on_exit } - return {} - end + local held = spawns.holding(function(argv) + return spawns.runs(argv, "gh") + end) local done = false fork_point.get_async(root, "feature", function() done = true end) fork_point.recheck(root, "feature") local answered = vim.wait(5000, function() - return asks == 2 and #held > 1 + return asks == 2 and #held.calls > 1 end, 10) - vim.system = real_system - for _, process in ipairs(held) do - real_system(unpack(process)) - end + held.release() assert.is_true(answered) assert.is_true(vim.wait(5000, function() @@ -360,7 +355,6 @@ describe("fork_point", function() it("measures again in the background when gh names a target while it measures", function() vim.env.FAKE_GH_PR = gh.pr_view({ baseRefName = "parent", number = 7 }) - vim.env.FAKE_GH_DELAY = "0.5" local root, _, parent_base = repo() local answered = false local pr_target = Git.pr_target @@ -371,14 +365,9 @@ describe("fork_point", function() end) end get(root, "feature") - local real_system, held = vim.system, {} - vim.system = function(argv, opts, on_exit) - if argv[1] == "gh" or answered then - return real_system(argv, opts, on_exit) - end - held[#held + 1] = { argv, opts, on_exit } - return {} - end + local held = spawns.holding(function(argv) + return answered or spawns.runs(argv, "gh") + end) local point ---@type changeset.ForkPoint? fork_point.get_async(root, "feature", function(landed) point = landed @@ -386,10 +375,7 @@ describe("fork_point", function() local done = vim.wait(5000, function() return answered end, 10) - vim.system = real_system - for _, process in ipairs(held) do - real_system(unpack(process)) - end + held.release() assert.is_true(done) assert.is_true(vim.wait(5000, function() @@ -450,13 +436,17 @@ describe("fork_point", function() end) it("asks gh once while an answer is in flight, and again after an answer of no PR", function() - vim.env.FAKE_GH_DELAY = "0.2" local root = repo() + -- Held until the second measure has landed, however long measuring takes under load. + held_gh = spawns.holding(function(argv) + return not spawns.runs(argv, "gh") + end) get(root, "feature") get(root, "feature") assert.equal(1, asks) + present(held_gh).release() assert.is_true(await_heard(root, 1)) get(root, "feature") assert.equal(2, asks) diff --git a/tests/git_spec.lua b/tests/git_spec.lua index 7b22f72..96b1724 100644 --- a/tests/git_spec.lua +++ b/tests/git_spec.lua @@ -1,5 +1,6 @@ local Git = require("changeset.git") local Fixture = require("support.git") +local Notify = require("support.notify") local present = require("support.present") describe("changeset.git", function() @@ -244,8 +245,25 @@ describe("changeset.git", function() return result end + local notes ---@type support.notify.Note[] + local restore_notify ---@type fun() + before_each(function() Fixture.init_repo("main", tmp) + notes, restore_notify = Notify.capture() + end) + + after_each(function() + restore_notify() + end) + + it("reads a command that fails to spawn as no lines", function() + assert.same( + {}, + run(function() + return Git.lines({ "changeset-no-such-program" }) + end) + ) end) it("reads a failed command's lines as none", function() @@ -284,69 +302,38 @@ describe("changeset.git", function() ) end) - it("raises an error raised inside fn, with its traceback", function() - local ok, err = pcall(Git.async, function() + it("reports an error fn raises after a git command, and calls back with nil", function() + local result = run(function() + Git.lines({ "git", "branch", "--show-current" }, tmp) error("measure failed") - end, function() end) + end) - assert.is_false(ok) - ---@cast err string - assert.matches("measure failed.*stack traceback", err) + assert.is_nil(result) + assert.matches( + "^Changeset: .*measure failed.*stack traceback", + present(Notify.messages(notes, vim.log.levels.ERROR)[1]) + ) end) - end) - describe("system", function() - it("hands a failed spawn to on_exit on the main loop as a failed result", function() - local real = vim.system - vim.system = function() - error("E2BIG") - end - local got ---@type vim.SystemCompleted? - local fast - Git.system({ "git", "status" }, {}, function(result) - got, fast = result, vim.in_fast_event() - end) - vim.system = real - assert.is_nil(got) - vim.wait(1000, function() - return got ~= nil + it("reports an error value fn raises that is not a string, and calls back with nil", function() + local result = run(function() + Git.lines({ "git", "branch", "--show-current" }, tmp) + error({ code = 1 }) end) - local result = present(got) - assert.equal(-1, result.code) - assert.matches("E2BIG", present(result.stderr)) - assert.is_false(fast) - end) - it("calls on_exit on the main loop with the process's result", function() - local got ---@type vim.SystemCompleted? - local fast - Git.system({ "git", "--version" }, { text = true }, function(result) - got, fast = result, vim.in_fast_event() - end) - vim.wait(5000, function() - return got ~= nil - end) - local result = present(got) - assert.equal(0, result.code) - assert.matches("^git version", present(result.stdout)) - assert.is_false(fast) + assert.is_nil(result) + assert.matches("^Changeset: table: .*stack traceback", present(Notify.messages(notes, vim.log.levels.ERROR)[1])) end) end) describe("pr_target", function() it("answers no PR when gh can't be started", function() require("support.gh") - local real = vim.system - vim.system = function() - error("E2BIG") - end local answered = false local target = "unset" ---@type string? - local ok = pcall(Git.pr_target, tmp, function(t) + require("support.spawns").failing(Git.pr_target, tmp, function(t) answered, target = true, t end) - vim.system = real - assert.is_true(ok) vim.wait(1000, function() return answered end) @@ -441,23 +428,23 @@ describe("changeset.git", function() end) describe("detached", function() - local real_systemlist, real_system, spawned + local real_systemlist, real_jobstart, spawned before_each(function() spawned = 0 - real_systemlist, real_system = vim.fn.systemlist, vim.system + real_systemlist, real_jobstart = vim.fn.systemlist, vim.fn.jobstart vim.fn.systemlist = function(...) spawned = spawned + 1 return real_systemlist(...) end - vim.system = function(...) + vim.fn.jobstart = function(...) spawned = spawned + 1 - return real_system(...) + return real_jobstart(...) end end) after_each(function() - vim.fn.systemlist, vim.system = real_systemlist, real_system + vim.fn.systemlist, vim.fn.jobstart = real_systemlist, real_jobstart end) it("is false on a branch", function() diff --git a/tests/herdr_spec.lua b/tests/herdr_spec.lua index a65e18e..e5c3b10 100644 --- a/tests/herdr_spec.lua +++ b/tests/herdr_spec.lua @@ -294,12 +294,7 @@ describe("changeset.herdr", function() end) it("refuses when herdr can't be started", function() - local system = vim.system - vim.system = function() - error("E2BIG") - end - local err, _, called = send("hi") - vim.system = system + local err, _, called = require("support.spawns").failing(send, "hi") assert.is_true(called) assert.truthy(err) end) diff --git a/tests/process_spec.lua b/tests/process_spec.lua new file mode 100644 index 0000000..0dbf58b --- /dev/null +++ b/tests/process_spec.lua @@ -0,0 +1,115 @@ +local Process = require("changeset.process") +local present = require("support.present") + +describe("changeset.process", function() + describe("run", function() + ---Run `argv` through `Process.run`, waiting for its result. + ---@param argv string[] + ---@param opts changeset.process.Opts + ---@return changeset.process.Completed + local function run(argv, opts) + local got ---@type changeset.process.Completed? + Process.run(argv, opts, function(result) + got = result + end) + assert.is_true(vim.wait(5000, function() + return got ~= nil + end, 10)) + return present(got) + end + + local real_system = vim.system + + -- A failing `run` raises before a spec could restore its stub. + after_each(function() + vim.system = real_system + end) + + it("starts its process without vim.system, whose spawn blocks Neovim longer", function() + local calls = 0 + vim.system = function(...) + calls = calls + 1 + return real_system(...) + end + + assert.equal(0, run({ "git", "--version" }, {}).code) + assert.equal(0, calls) + end) + + it("hands a failed spawn to its callback on the main loop as a failed result", function() + local got ---@type changeset.process.Completed? + local fast + require("support.spawns").failing(Process.run, { "git", "status" }, {}, function(result) + got, fast = result, vim.in_fast_event() + end) + assert.is_nil(got) + vim.wait(1000, function() + return got ~= nil + end) + local result = present(got) + assert.equal(-1, result.code) + assert.matches("E2BIG", present(result.stderr)) + assert.is_false(fast) + end) + + it("calls back on the main loop with the process's result", function() + local got ---@type changeset.process.Completed? + local fast + Process.run({ "git", "--version" }, {}, function(result) + got, fast = result, vim.in_fast_event() + end) + vim.wait(5000, function() + return got ~= nil + end) + local result = present(got) + assert.equal(0, result.code) + assert.matches("^git version", present(result.stdout)) + assert.is_false(fast) + end) + + it("keeps NUL bytes in stdout", function() + assert.equal("a\0b\n", run({ "printf", "a\\0b\\n" }, {}).stdout) + end) + + it("reads CRLF output as LF", function() + assert.equal("a\nb\n", run({ "printf", "a\\r\\nb\\r\\n" }, {}).stdout) + end) + + it("writes stdin to the process with its NUL bytes", function() + assert.equal("x\0y\nz", run({ "cat" }, { stdin = "x\0y\nz" }).stdout) + end) + + it("captures a failing command's stderr", function() + local result = run({ "sh", "-c", "echo oops >&2; exit 3" }, {}) + assert.equal(3, result.code) + assert.equal("oops\n", result.stderr) + end) + + it("stops a command that outlives its timeout and reports code 124", function() + local start = vim.uv.now() + assert.equal(124, run({ "sleep", "5" }, { timeout = 50 }).code) + assert.is_true(vim.uv.now() - start < 2000) + end) + + it("reports the exit code of a command that exits as its timeout fires", function() + local real_jobstart, real_defer = vim.fn.jobstart, vim.defer_fn + local timeout ---@type fun()? + vim.defer_fn = function(fn) + timeout = fn + return present(vim.uv.new_timer()) + end + vim.fn.jobstart = function(argv, opts) + local on_exit = present(opts).on_exit + present(opts).on_exit = function(...) + present(timeout)() + return on_exit(...) + end + return real_jobstart(argv, opts) + end + local ok, result = pcall(run, { "true" }, { timeout = 5000 }) + vim.fn.jobstart, vim.defer_fn = real_jobstart, real_defer + assert.is_true(ok, tostring(result)) + assert.equal(0, result.code) + end) + end) +end) diff --git a/tests/review_spec.lua b/tests/review_spec.lua index 9a1ec3d..b198311 100644 --- a/tests/review_spec.lua +++ b/tests/review_spec.lua @@ -3,6 +3,7 @@ local support = require("support.git") local gutter = require("support.gutter") local Notify = require("support.notify") local present = require("support.present") +local spawns = require("support.spawns") local await, edit, revision = gutter.await, gutter.edit, gutter.revision @@ -13,6 +14,7 @@ describe("the gutter's base", function() local change_base ---@type { msg: string, level: integer? }[] local notices + local held ---@type support.spawns.Held? ---A repo with `a.txt` changed on `parent`, then again on `child` cut from it. ---@param child string @@ -33,9 +35,12 @@ describe("the gutter's base", function() end) after_each(function() + if held then + held.stop() + held = nil + end gutter.teardown(dir, cwd) vim.env.FAKE_GH_PR = nil - vim.env.FAKE_GH_DELAY = nil restore_notify() require("gitsigns").change_base = change_base end) @@ -170,23 +175,33 @@ describe("the gutter's base", function() it("drops a PR lookup that a branch switch superseded", function() stack("stacked-switched") - vim.env.FAKE_GH_PR = gh.pr_view({ baseRefName = "parent" }) - vim.env.FAKE_GH_DELAY = "1" + -- main moves on past where parent forked, so main's base differs from its fork point with parent. + support.git({ "switch", "-q", "main" }, dir) + gutter.advance(dir) + support.git({ "switch", "-q", "stacked-switched" }, dir) vim.fn.chdir(dir) + held = spawns.holding(function(argv) + return not spawns.runs(argv, "gh") + end) local bufs = edit({ "a.txt" }) assert.is_true(await(bufs, gutter.merge_base(dir), 5000)) - local parent_tip = support.git({ "rev-parse", "parent" }, dir) + -- Only the child's lookup stays held; main's runs once the switch asks it, and finds no PR. + present(held).stop() + assert.equal(1, #present(held).calls) -- The child's lookup is still in flight when the switch happens. Its answer names the parent, and must - -- not move main's buffers onto it. - vim.env.FAKE_GH_PR = nil + -- not move main's buffers off main's base. support.git({ "switch", "-q", "main" }, dir) - assert.is_true(vim.wait(10000, function() - return vim.b[bufs[1]].gitsigns_head == "main" - end, 20)) + local main_base = gutter.merge_base(dir) + -- main's own lookup has started by the time its base lands, so only the child's reads the PR set below. + assert.is_true(await(bufs, main_base, 10000)) + -- gh reads $FAKE_GH_PR as it starts, which a held lookup does only on release. + vim.env.FAKE_GH_PR = gh.pr_view({ baseRefName = "parent" }) + present(held).release() + vim.env.FAKE_GH_PR = nil assert.is_false(vim.wait(1500, function() - return revision(present(bufs[1])) == parent_tip + return revision(present(bufs[1])) ~= main_base end, 20)) end) end) diff --git a/tests/review_switch_spec.lua b/tests/review_switch_spec.lua index bcbc6a9..91bacd6 100644 --- a/tests/review_switch_spec.lua +++ b/tests/review_switch_spec.lua @@ -33,6 +33,7 @@ end describe("the gutter's base", function() local dir ---@type string local cwd ---@type string + local held_gh ---@type support.spawns.Held? before_each(function() cwd = vim.fn.getcwd() @@ -40,8 +41,12 @@ describe("the gutter's base", function() end) after_each(function() + if held_gh then + held_gh.stop() + held_gh = nil + end gutter.teardown(dir, cwd) - vim.env.FAKE_GH_PR, vim.env.FAKE_GH_DELAY, vim.env.FAKE_GH_LOG = nil, nil, nil + vim.env.FAKE_GH_PR, vim.env.FAKE_GH_LOG = nil, nil end) it("moves attached buffers to the new merge base after an external branch switch", function() @@ -273,8 +278,11 @@ describe("the gutter's base", function() vim.fn.writefile({ "one" }, dir .. "/a.txt") local pushed = support.commit("pushed", dir) support.git({ "update-ref", "refs/remotes/origin/main", pushed }, dir) - vim.env.FAKE_GH_DELAY = "2" vim.fn.chdir(dir) + -- Held until HEAD has moved the base, however long that takes under load. + held_gh = spawns.holding(function(argv) + return not spawns.runs(argv, "gh") + end) local bufs = edit({ "a.txt" }) assert.is_true(await(bufs, pushed, 5000)) @@ -282,6 +290,7 @@ describe("the gutter's base", function() local pulled = support.commit("pulled", dir) support.git({ "update-ref", "refs/remotes/origin/main", pulled }, dir) assert.is_true(await(bufs, pulled, 10000)) + present(held_gh).release() assert.is_false(vim.wait(3000, function() return revision(present(bufs[1])) ~= pulled diff --git a/tests/support/gh.lua b/tests/support/gh.lua index 906baed..670381d 100644 --- a/tests/support/gh.lua +++ b/tests/support/gh.lua @@ -1,13 +1,12 @@ ----A fake `gh` on PATH, so no spec asks GitHub about its fixture branch: it prints ----$FAKE_GH_PR after $FAKE_GH_DELAY seconds, and fails as if there were no PR when ----that is empty. Each call appends a line to $FAKE_GH_LOG when that is set. Requiring it is what installs it; PATH is never restored, since ----each spec runs in its own nvim. `without` hides it, for a case where gh is not installed. +---A fake `gh` on PATH, so no spec asks GitHub about its fixture branch: it prints $FAKE_GH_PR, and fails as if there +---were no PR when that is empty. Each call appends a line to $FAKE_GH_LOG when that is set. Requiring it is what +---installs it; PATH is never restored, since each spec runs in its own nvim. `without` hides it, for a case where gh is +---not installed. local bin = vim.fn.tempname() vim.fn.mkdir(bin, "p") vim.fn.writefile({ "#!/bin/sh", '[ -z "$FAKE_GH_LOG" ] || echo "$*" >> "$FAKE_GH_LOG"', - 'sleep "${FAKE_GH_DELAY:-0}"', '[ -n "$FAKE_GH_PR" ] || exit 1', 'printf "%s" "$FAKE_GH_PR"', }, bin .. "/gh") diff --git a/tests/support/spawns.lua b/tests/support/spawns.lua index 9039c5c..7e6d5b0 100644 --- a/tests/support/spawns.lua +++ b/tests/support/spawns.lua @@ -1,4 +1,5 @@ ----Records the processes code starts, to tell what it spawned and whether that blocked Neovim. +---Records, holds or fails the processes code starts, to tell what it spawned, whether that blocked Neovim, what +---happens before it runs, and what happens when it can't start. local M = {} @@ -24,12 +25,12 @@ local function spying(targets, fn) return argvs end ----Runs `fn`, keeping the argv of every process started through `vim.system` or `vim.fn.systemlist`. +---Runs `fn`, keeping the argv of every process started through `vim.system`, `vim.fn.jobstart` or `vim.fn.systemlist`. ---A wait belongs inside `fn`: `check-attr` starts from a scheduled callback, after `build()` returns. ---@param fn fun() ---@return string[][] function M.recording(fn) - return spying({ { vim, "system" }, { vim.fn, "systemlist" } }, fn) + return spying({ { vim, "system" }, { vim.fn, "jobstart" }, { vim.fn, "systemlist" } }, fn) end ---Runs `fn`, keeping the argv of every process started through `vim.fn.systemlist` or `vim.fn.system`, which block @@ -40,6 +41,63 @@ function M.blocking(fn) return spying({ { vim.fn, "system" }, { vim.fn, "systemlist" } }, fn) end +---The calls `holding` keeps unstarted, and what can be done with them. +---@class support.spawns.Held +---@field calls fun()[] Each starts its held call. +---@field stop fun() Stops holding, starting nothing; safe to call twice. +---@field release fun() Stops holding, then starts every held call in the order it was held. + +---Holds every call to `Process.run` unstarted from now on, letting through those `pass` accepts. +---@param pass (fun(argv: string[]): boolean)? +---@return support.spawns.Held +function M.holding(pass) + local Process = require("changeset.process") + local real = Process.run + local held ---@type support.spawns.Held + held = { + calls = {}, + stop = function() + Process.run = real + end, + release = function() + held.stop() + local calls = held.calls + held.calls = {} + for _, call in ipairs(calls) do + call() + end + end, + } + Process.run = function(argv, opts, on_result) + if pass and pass(argv) then + return real(argv, opts, on_result) + end + table.insert(held.calls, function() + real(argv, opts, on_result) + end) + end + return held +end + +---Run `fn(...)` with every process failing to start, as an argv over the OS's limit does. `vim.fn.jobstart` comes +---back even when `fn` errors. +---@generic T +---@param fn fun(...): T +---@return T +---@return any ... Any further values `fn` returns. +function M.failing(fn, ...) + local real = vim.fn.jobstart + vim.fn.jobstart = function() + error("E2BIG") + end + local res = vim.F.pack_len(pcall(fn, ...)) + vim.fn.jobstart = real + if not res[1] then + error(res[2], 0) + end + return unpack(res, 2, res.n) +end + ---Whether `argv` starts `program` with `subcommand`, as `git -C merge-base …` starts git's merge-base. ---@param argv string[] ---@param program string diff --git a/tests/tree_lifecycle_spec.lua b/tests/tree_lifecycle_spec.lua index 4bb7631..c4d60a9 100644 --- a/tests/tree_lifecycle_spec.lua +++ b/tests/tree_lifecycle_spec.lua @@ -8,6 +8,7 @@ local Notify = require("support.notify") local Sidebar = require("support.sidebar") local gh = require("support.gh") local present = require("support.present") +local spawns = require("support.spawns") ---@param tree changeset.Tree? ---@return string[] @@ -333,18 +334,21 @@ describe("changeset tree", function() end) describe("reopened on a kept tree", function() - local real_system ---@type function local real_systemlist ---@type function - local held ---@type table[] + local held ---@type support.spawns.Held? local waited - ---Hold every `vim.system` process unstarted, and count the git processes Neovim waits on. - local function hold() - held, waited = {}, 0 - vim.system = function(argv, opts, on_exit) - held[#held + 1] = { argv, opts, on_exit } - return {} + ---Put back every spawner `hold` replaced. + local function unhold() + vim.fn.systemlist = real_systemlist + if held then + held.stop() end + end + + ---Hold every `Process.run` process unstarted, and count the git processes Neovim waits on. + local function hold() + held, waited = spawns.holding(), 0 vim.fn.systemlist = function(...) waited = waited + 1 return real_systemlist(...) @@ -353,21 +357,18 @@ describe("changeset tree", function() ---Start every held process, now. local function release() - vim.system, vim.fn.systemlist = real_system, real_systemlist - for _, process in ipairs(held) do - real_system(unpack(process)) - end - held = {} + unhold() + present(held).release() end before_each(function() - real_system, real_systemlist = vim.system, vim.fn.systemlist + real_systemlist, held = vim.fn.systemlist, nil Build.build() assert.is_true(wait_for_file("mod.lua")) end) after_each(function() - vim.system, vim.fn.systemlist = real_system, real_systemlist + unhold() end) it("draws the kept tree before waiting on git", function() @@ -428,7 +429,7 @@ describe("changeset tree", function() it("builds for the branch HEAD moved to before the re-measure landed", function() hold() changeset.open() - vim.system, vim.fn.systemlist = real_system, real_systemlist + unhold() Fixture.git({ "checkout", "-q", "-b", "feature2" }, tmp) release() @@ -441,7 +442,7 @@ describe("changeset tree", function() it("keeps the tree a build for another branch made before the re-measure landed", function() hold() changeset.open() - vim.system, vim.fn.systemlist = real_system, real_systemlist + unhold() Fixture.git({ "checkout", "-q", "-b", "feature2" }, tmp) Build.build() local tree = present(build.current())