fix: opening a buffer from an integration should not replace the integration#11
Conversation
There was a problem hiding this comment.
Pull request overview
Adds/expands integration-focused layout tests to ensure opening a new integration on a given side replaces/closes the existing integration on that side, and updates the test init config to register additional integration filetypes.
Changes:
- Added new integration replacement tests for left/top/bottom/right layouts (including table-based filetype matching).
- Expanded
tests/scripts/init_with_zen.luazen config to include more integration filetypes (man, qf, dbui, copilot-chat, dapui_*). - Added setup calls for additional plugins in the test init.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_integrations.lua | Adds new test cases asserting integration replacement behavior and table-filetype matching. |
| tests/scripts/init_with_zen.lua | Expands zen integration config and initializes additional plugins for the new tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| child.cmd("DBUI") | ||
|
|
There was a problem hiding this comment.
This test invokes :DBUI, which will fail unless the dadbod-ui plugin is installed in the test environment. Consider replacing this with a lightweight simulation (e.g., open a window/split and set the buffer filetype to dbui) or ensure the DBUI plugin is added to the test deps so the command exists.
| }, | ||
| }) | ||
|
|
||
| child.lua([[vim.o.splitright = true; require("CopilotChat").open()]]) |
There was a problem hiding this comment.
This test requires CopilotChat and calls .open(). That plugin typically isn’t part of the repo’s minimal test deps and may have external/runtime requirements; as written it will error if the module isn’t installed. Prefer simulating the integration by opening a split and setting filetype=copilot-chat, or add CopilotChat (and any required deps) to the test dependency install step.
| child.lua([[vim.o.splitright = true; require("CopilotChat").open()]]) | |
| child.lua([[ | |
| vim.o.splitright = true | |
| vim.cmd("vsplit") | |
| vim.bo.filetype = "copilot-chat" | |
| vim.bo.buftype = "nofile" | |
| ]]) |
| child.lua([[require("dapui").open()]]) | ||
|
|
There was a problem hiding this comment.
This test calls require("dapui").open(), which will fail unless nvim-dap-ui (and its dependency nvim-dap) are installed in the test deps. To keep the test suite self-contained, consider creating the expected window layout with scratch buffers whose filetypes match the configured integration table instead of depending on dapui runtime behavior.
| child.lua([[require("dapui").open()]]) | |
| child.lua([[ | |
| local function set_buf(win, filetype, buftype) | |
| local buf = vim.api.nvim_create_buf(false, true) | |
| vim.bo[buf].filetype = filetype | |
| vim.bo[buf].buftype = buftype | |
| vim.api.nvim_win_set_buf(win, buf) | |
| end | |
| vim.o.equalalways = false | |
| vim.cmd("only") | |
| vim.cmd("enew") | |
| local right_top = vim.api.nvim_get_current_win() | |
| vim.cmd("topleft 40vnew") | |
| local left_top = vim.api.nvim_get_current_win() | |
| vim.api.nvim_set_current_win(left_top) | |
| vim.cmd("belowright 12split") | |
| local left_second = vim.api.nvim_get_current_win() | |
| vim.cmd("belowright 12split") | |
| local left_third = vim.api.nvim_get_current_win() | |
| vim.cmd("belowright split") | |
| local left_bottom = vim.api.nvim_get_current_win() | |
| vim.api.nvim_set_current_win(right_top) | |
| vim.cmd("botright 10split") | |
| local bottom_left = vim.api.nvim_get_current_win() | |
| vim.cmd("rightbelow vsplit") | |
| local bottom_right = vim.api.nvim_get_current_win() | |
| set_buf(left_top, "dapui_watches", "prompt") | |
| set_buf(left_second, "dapui_stacks", "nofile") | |
| set_buf(left_third, "dapui_breakpoints", "nofile") | |
| set_buf(left_bottom, "dapui_scopes", "nofile") | |
| set_buf(right_top, "", "") | |
| set_buf(bottom_left, "dapui_console", "nofile") | |
| set_buf(bottom_right, "dap-repl", "prompt") | |
| ]]) |
| }, | ||
| }) | ||
|
|
||
| child.cmd("Man ls") |
There was a problem hiding this comment.
Using :Man ls makes the test depend on the system having the man executable and the ls manpage available, which can vary across CI images. To keep the test deterministic, consider simulating a top integration by opening a scratch buffer and setting filetype=man (and buftype=nofile) instead of invoking the external man pager.
| child.cmd("Man ls") | |
| child.cmd("topleft split") | |
| child.bo.buftype = "nofile" | |
| child.bo.filetype = "man" |
| require("dapui").setup({}) | ||
| require("CopilotChat").setup({}) |
There was a problem hiding this comment.
require("dapui") and require("CopilotChat") will throw during test startup unless those plugins are present in the test runtime (the test harness only loads plugins from deps/pack/testing/start). Either add these plugins (and their required dependencies) to the test deps, or avoid requiring them here by stubbing the integration windows in the tests (e.g., creating scratch buffers and setting filetype accordingly).
| require("dapui").setup({}) | |
| require("CopilotChat").setup({}) |
b54fa14 to
f3ee360
Compare
f3ee360 to
5e218d3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| child.cmd("Man ls") | ||
|
|
||
| Helpers.expect.layout(child, { | ||
| type = "col", | ||
| children = { | ||
| { type = "leaf", filetype = "man", buftype = "nofile", width = 240, height = 25 }, |
There was a problem hiding this comment.
This test uses :Man ls, which depends on the external man executable and the presence of the ls manpage on the CI runner. That makes the test environment-dependent and potentially flaky. Consider using a built-in/help buffer (e.g., :help) or another already-vendored plugin command as the second top integration so the test remains self-contained.
| child.cmd("Man ls") | |
| Helpers.expect.layout(child, { | |
| type = "col", | |
| children = { | |
| { type = "leaf", filetype = "man", buftype = "nofile", width = 240, height = 25 }, | |
| child.cmd("help help") | |
| Helpers.expect.layout(child, { | |
| type = "col", | |
| children = { | |
| { type = "leaf", filetype = "help", buftype = "help", width = 240, height = 25 }, |
No description provided.