Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/lua.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ jobs:
github_access_token: ${{ secrets.GITHUB_TOKEN }}
- name: Validate Neovim Configuration (Dev Shell)
run: make lua-check-neovim-dev
lua-neovim-test:
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@v6
- name: Install Nix
uses: cachix/install-nix-action@v31
with:
github_access_token: ${{ secrets.GITHUB_TOKEN }}
- name: Run Neovim Tests (Dev Shell)
run: make neovim-test-dev
lua-hammerspoon:
runs-on: macos-latest
timeout-minutes: 300
Expand All @@ -38,6 +50,7 @@ jobs:
if: always()
needs:
- lua-neovim
- lua-neovim-test
- lua-hammerspoon
runs-on: ubuntu-latest
timeout-minutes: 3
Expand Down
11 changes: 11 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,17 @@ neovim-sync: ## Sync Neovim plugins.
@nvim --headless +"lua vim.cmd('source ' .. vim.fn.stdpath('config') .. '/init.lua')" +qa
@echo "✅ Neovim plugins synced"

.PHONY: neovim-test
neovim-test: ## Run Neovim tests using plenary.nvim.
@echo "🧪 Running Neovim tests..."
@$(PWD)/home-manager/programs/neovim/tests/run_tests.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with other parts of the Makefile (e.g., line 503) and for better portability, it's recommended to use $(CURDIR) instead of $(PWD) to refer to the current directory.

	@$(CURDIR)/home-manager/programs/neovim/tests/run_tests.sh

@echo "✅ Neovim tests completed"
Comment on lines +497 to +498

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The success message is printed even if the test script fails. Move the success message into the conditional check based on the exit code, or remove it since the script itself already prints success/failure messages.

Suggested change
@$(PWD)/home-manager/programs/neovim/tests/run_tests.sh
@echo "✅ Neovim tests completed"
@$(PWD)/home-manager/programs/neovim/tests/run_tests.sh && echo "✅ Neovim tests completed"

Copilot uses AI. Check for mistakes.

.PHONY: neovim-test-dev
neovim-test-dev: ## Run Neovim tests inside the Nix dev shell (mirrors CI).
@echo "🧪 Running Neovim tests inside the Nix dev shell..."
@DEVENV_ROOT=$(CURDIR) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) develop $(NIX_FLAGS) .# --command $(MAKE) neovim-test

##@ Lua

.PHONY: lua-check
Expand Down
63 changes: 63 additions & 0 deletions home-manager/programs/neovim/tests/config_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
-- Tests for Neovim configuration loading
-- Verifies that the config modules can be loaded without errors

describe("config", function()
describe("settings", function()
it("should load without errors", function()
assert.has_no.errors(function()
package.loaded["settings"] = nil
require("settings")
end)
end)
end)

describe("keymaps", function()
it("should define leader key", function()
assert.equals(" ", vim.g.mapleader)
end)
end)

describe("nvim API", function()
it("should have working buffer API", function()
local buf = vim.api.nvim_get_current_buf()
assert.is_number(buf)
assert.is_true(vim.api.nvim_buf_is_valid(buf))
end)

it("should have working window API", function()
local win = vim.api.nvim_get_current_win()
assert.is_number(win)
assert.is_true(vim.api.nvim_win_is_valid(win))
end)

it("should be able to create and manipulate buffers", function()
-- Create a new buffer
local buf = vim.api.nvim_create_buf(false, true)
assert.is_true(vim.api.nvim_buf_is_valid(buf))

-- Set lines
vim.api.nvim_buf_set_lines(buf, 0, -1, false, { "hello", "world" })
local lines = vim.api.nvim_buf_get_lines(buf, 0, -1, false)
assert.are.same({ "hello", "world" }, lines)

-- Clean up
vim.api.nvim_buf_delete(buf, { force = true })
end)

it("should handle feedkeys for keymap testing", function()
-- Create a scratch buffer for testing
local buf = vim.api.nvim_create_buf(false, true)
vim.api.nvim_set_current_buf(buf)

-- Enter insert mode and type
vim.api.nvim_feedkeys("itest", "x", false)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High

Race condition: nvim_feedkeys() with the "x" flag processes keys asynchronously, but line 56 immediately checks the result without synchronization. This test will be flaky.

The feedkeys are queued but may not be processed before nvim_get_current_line() executes. Use synchronous buffer operations instead:

local buf = vim.api.nvim_create_buf(false, true)
vim.api.nvim_set_current_buf(buf)
vim.api.nvim_buf_set_lines(buf, 0, -1, false, {"test"})
local line = vim.api.nvim_buf_get_lines(buf, 0, 1, false)[1]
assert.equals("test", line)
vim.api.nvim_buf_delete(buf, { force = true })

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#395
File: home-manager/programs/neovim/tests/config_spec.lua#L53
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Race condition: `nvim_feedkeys()` with the `"x"` flag processes keys asynchronously, but line 56 immediately checks the result without synchronization. This test will be flaky.

The feedkeys are queued but may not be processed before `nvim_get_current_line()` executes. Use synchronous buffer operations instead:

```lua
local buf = vim.api.nvim_create_buf(false, true)
vim.api.nvim_set_current_buf(buf)
vim.api.nvim_buf_set_lines(buf, 0, -1, false, {"test"})
local line = vim.api.nvim_buf_get_lines(buf, 0, 1, false)[1]
assert.equals("test", line)
vim.api.nvim_buf_delete(buf, { force = true })

</details>

vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<Esc>", true, false, true), "x", false)

local line = vim.api.nvim_get_current_line()
assert.equals("test", line)

-- Clean up
vim.api.nvim_buf_delete(buf, { force = true })
end)
end)
Comment on lines +20 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The cleanup logic (e.g., vim.api.nvim_buf_delete) in these tests will not execute if an assertion fails. This can lead to leftover buffers that might interfere with subsequent tests. It's more robust to handle cleanup in an after_each block to ensure it always runs.

	describe("nvim API", function()
		local bufs_to_clean = {}
		after_each(function()
			for _, b in ipairs(bufs_to_clean) do
				if vim.api.nvim_buf_is_valid(b) then
					vim.api.nvim_buf_delete(b, { force = true })
				end
			end
			bufs_to_clean = {}
		end)

		it("should have working buffer API", function()
			local buf = vim.api.nvim_get_current_buf()
			assert.is_number(buf)
			assert.is_true(vim.api.nvim_buf_is_valid(buf))
		end)

		it("should have working window API", function()
			local win = vim.api.nvim_get_current_win()
			assert.is_number(win)
			assert.is_true(vim.api.nvim_win_is_valid(win))
		end)

		it("should be able to create and manipulate buffers", function()
			-- Create a new buffer
			local buf = vim.api.nvim_create_buf(false, true)
			table.insert(bufs_to_clean, buf)
			assert.is_true(vim.api.nvim_buf_is_valid(buf))

			-- Set lines
			vim.api.nvim_buf_set_lines(buf, 0, -1, false, { "hello", "world" })
			local lines = vim.api.nvim_buf_get_lines(buf, 0, -1, false)
			assert.are.same({ "hello", "world" }, lines)
		end)

		it("should handle feedkeys for keymap testing", function()
			-- Create a scratch buffer for testing
			local buf = vim.api.nvim_create_buf(false, true)
			table.insert(bufs_to_clean, buf)
			vim.api.nvim_set_current_buf(buf)

			-- Enter insert mode and type
			vim.api.nvim_feedkeys("itest", "x", false)
			vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes("<Esc>", true, false, true), "x", false)

			local line = vim.api.nvim_get_current_line()
			assert.equals("test", line)
		end)
	end)

end)
49 changes: 49 additions & 0 deletions home-manager/programs/neovim/tests/minimal_init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
-- Minimal init.lua for Neovim testing
-- This provides a clean environment isolated from the full config

-- Set leader key (required by some tests)
vim.g.mapleader = " "
vim.g.maplocalleader = " "

-- Get the directory of this init file
local init_path = debug.getinfo(1, "S").source:sub(2)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium

Using debug.getinfo(1, "S").source:sub(2) for path detection is fragile:

  • May break if file is sourced differently (e.g., via :source, -u flag variations)
  • Symlinks could cause issues (relevant since home-manager uses symlinks heavily)
  • Hard to debug when it fails

Consider a more robust approach:

local tests_dir = vim.fn.expand("<sfile>:p:h")

Or set an environment variable in run_tests.sh and use vim.env.TEST_DIR.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#395
File: home-manager/programs/neovim/tests/minimal_init.lua#L9
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Using `debug.getinfo(1, "S").source:sub(2)` for path detection is fragile:
- May break if file is sourced differently (e.g., via `:source`, `-u` flag variations)
- Symlinks could cause issues (relevant since home-manager uses symlinks heavily)
- Hard to debug when it fails

Consider a more robust approach:
```lua
local tests_dir = vim.fn.expand("<sfile>:p:h")

Or set an environment variable in run_tests.sh and use vim.env.TEST_DIR.


</details>

local tests_dir = vim.fn.fnamemodify(init_path, ":h")
local nvim_dir = vim.fn.fnamemodify(tests_dir, ":h")

-- Add lua module paths
package.path = package.path .. ";" .. nvim_dir .. "/lua/?.lua"
package.path = package.path .. ";" .. nvim_dir .. "/lua/?/init.lua"

-- Set up runtimepath to include plenary
vim.opt.runtimepath:append(".")
vim.opt.runtimepath:append(nvim_dir)

-- Add plenary to runtimepath if installed via vim.pack or available
local plenary_paths = {
vim.fn.stdpath("data") .. "/site/pack/plugins/start/plenary.nvim",
vim.fn.stdpath("data") .. "/site/pack/plugins/opt/plenary.nvim",
vim.fn.expand("~/.local/share/nvim/site/pack/plugins/start/plenary.nvim"),
vim.fn.expand("~/.local/share/nvim/site/pack/plugins/opt/plenary.nvim"),
-- CI/test environment paths
"/tmp/plenary.nvim",
tests_dir .. "/plenary.nvim",
}
Comment on lines +22 to +30

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plenary path list includes hardcoded paths like ~/.local/share/nvim/site/pack/plugins/start/plenary.nvim which duplicate the logic of vim.fn.stdpath("data") paths on lines 23-24. Consider simplifying by removing the hardcoded home directory paths since stdpath("data") already resolves to the appropriate location.

Copilot uses AI. Check for mistakes.

for _, path in ipairs(plenary_paths) do
if vim.fn.isdirectory(path) == 1 then
vim.opt.runtimepath:append(path)
break
end
Comment on lines +28 to +36

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test setup adds /tmp/plenary.nvim to runtimepath and will execute Lua code from there if the directory exists, which allows arbitrary code execution from a world-writable location. A local attacker can pre-create /tmp/plenary.nvim with a malicious plenary plugin so that when run_tests.sh invokes Neovim and require('plenary.test_harness'), their code runs under the test user's privileges. To fix this, avoid loading plugins from /tmp (e.g., use a repo-local tests/plenary.nvim or a per-user stdpath('data') directory) and ensure any test-only plugin directory is created in a location not writable by other users.

Copilot uses AI. Check for mistakes.
end
Comment on lines +22 to +37

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The run_tests.sh script exports a PLENARY_DIR environment variable, but this file doesn't use it, relying on a hardcoded path /tmp/plenary.nvim. To make the test setup more flexible and honor the environment variable, you can read it using os.getenv(). This also requires a check to ensure the path is not nil before using it.

local plenary_env_path = os.getenv("PLENARY_DIR")
local plenary_paths = {
	-- CI/test environment paths
	plenary_env_path,
	"/tmp/plenary.nvim",
	tests_dir .. "/plenary.nvim",
	-- User installation paths
	vim.fn.stdpath("data") .. "/site/pack/plugins/start/plenary.nvim",
	vim.fn.stdpath("data") .. "/site/pack/plugins/opt/plenary.nvim",
	vim.fn.expand("~/.local/share/nvim/site/pack/plugins/start/plenary.nvim"),
	vim.fn.expand("~/.local/share/nvim/site/pack/plugins/opt/plenary.nvim"),
}

for _, path in ipairs(plenary_paths) do
	if path and vim.fn.isdirectory(path) == 1 then
		vim.opt.runtimepath:append(path)
		break
	end
end


-- Basic settings for testing
vim.opt.swapfile = false
vim.opt.backup = false
vim.opt.writebackup = false
vim.opt.undofile = false

-- Disable shada (shared data) to avoid file conflicts in parallel tests
vim.opt.shadafile = "NONE"

-- Set a reasonable timeout for async operations
vim.opt.updatetime = 100
51 changes: 51 additions & 0 deletions home-manager/programs/neovim/tests/run_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env bash
# Neovim Test Runner
# Runs plenary-based tests for the Neovim configuration

set -e

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
NVIM_DIR="$(dirname "$SCRIPT_DIR")"
PLENARY_DIR="${PLENARY_DIR:-/tmp/plenary.nvim}"

# Colors for output
RED='\033[0;31m'
GREEN='\033[0;32m'
YELLOW='\033[1;33m'
NC='\033[0m' # No Color

echo -e "${YELLOW}Running Neovim tests...${NC}"

# Check if nvim is available
if ! command -v nvim &>/dev/null; then
echo -e "${RED}Error: Neovim is not installed or not in PATH${NC}"
exit 1
fi

# Ensure plenary.nvim is available
if [ ! -d "$PLENARY_DIR" ]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High

The plenary.nvim dependency is cloned imperatively without version pinning, creating several issues:

  1. Non-deterministic: Using --depth 1 without a version/tag means tests could break if plenary updates
  2. Network dependency: Tests fail if GitHub is unreachable
  3. Inefficient: Re-clones on every CI run instead of caching
  4. Architectural inconsistency: This Nix-based dotfiles repo should manage dependencies declaratively

Recommendations:

  • Pin to a specific tag: git clone --depth 1 --branch v0.1.4 https://...
  • Add validation after clone to detect partial failures:
    if [ ! -f "$PLENARY_DIR/lua/plenary/test_harness.lua" ]; then
      echo "Error: plenary.nvim clone incomplete"
      rm -rf "$PLENARY_DIR"
      exit 1
    fi
  • Consider adding plenary.nvim to the Nix devshell in devenv.nix for proper declarative dependency management

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#395
File: home-manager/programs/neovim/tests/run_tests.sh#L26
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
The plenary.nvim dependency is cloned imperatively without version pinning, creating several issues:

1. **Non-deterministic**: Using `--depth 1` without a version/tag means tests could break if plenary updates
2. **Network dependency**: Tests fail if GitHub is unreachable
3. **Inefficient**: Re-clones on every CI run instead of caching
4. **Architectural inconsistency**: This Nix-based dotfiles repo should manage dependencies declaratively

Recommendations:
- Pin to a specific tag: `git clone --depth 1 --branch v0.1.4 https://...`
- Add validation after clone to detect partial failures:
  ```bash
  if [ ! -f "$PLENARY_DIR/lua/plenary/test_harness.lua" ]; then
    echo "Error: plenary.nvim clone incomplete"
    rm -rf "$PLENARY_DIR"
    exit 1
  fi
  • Consider adding plenary.nvim to the Nix devshell in devenv.nix for proper declarative dependency management

</details>

echo -e "${YELLOW}Cloning plenary.nvim to $PLENARY_DIR...${NC}"
git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"
Comment on lines +27 to +28

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script clones plenary.nvim from GitHub without verifying the repository's authenticity or pinning to a specific commit/tag. Consider adding a version pin (e.g., --branch v0.1.4) to ensure reproducible test runs and avoid potential issues from upstream changes.

Suggested change
echo -e "${YELLOW}Cloning plenary.nvim to $PLENARY_DIR...${NC}"
git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"
echo -e "${YELLOW}Cloning plenary.nvim (tag v0.1.4) to $PLENARY_DIR...${NC}"
git clone --branch v0.1.4 --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"

Copilot uses AI. Check for mistakes.
fi
Comment on lines +26 to +29

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Verify git clone success before proceeding.

The script clones plenary.nvim but doesn't verify the clone succeeded. If the clone fails, subsequent test execution may fail with unclear errors.

Apply this diff:

 if [ ! -d "$PLENARY_DIR" ]; then
   echo -e "${YELLOW}Cloning plenary.nvim to $PLENARY_DIR...${NC}"
-  git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"
+  if ! git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"; then
+    echo -e "${RED}Error: Failed to clone plenary.nvim${NC}"
+    exit 1
+  fi
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ ! -d "$PLENARY_DIR" ]; then
echo -e "${YELLOW}Cloning plenary.nvim to $PLENARY_DIR...${NC}"
git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"
fi
if [ ! -d "$PLENARY_DIR" ]; then
echo -e "${YELLOW}Cloning plenary.nvim to $PLENARY_DIR...${NC}"
if ! git clone --depth 1 https://github.com/nvim-lua/plenary.nvim "$PLENARY_DIR"; then
echo -e "${RED}Error: Failed to clone plenary.nvim${NC}"
exit 1
fi
fi
🤖 Prompt for AI Agents
In home-manager/programs/neovim/tests/run_tests.sh around lines 26 to 29, the
script clones plenary.nvim but does not check whether git clone succeeded;
update the block to verify the clone command returned success and if it failed
print a clear error (including the target dir and git error context) to stderr
and exit with a non-zero status so tests don't continue on a failed clone (e.g.,
run git clone and immediately check its exit code or test that the directory now
exists and on failure echo an error to >&2 and exit 1).


# Export plenary directory for minimal_init.lua
export PLENARY_DIR

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PLENARY_DIR environment variable is exported but not actually used by the minimal_init.lua script. The minimal_init.lua searches for plenary in hardcoded paths including /tmp/plenary.nvim, but doesn't check the PLENARY_DIR environment variable. Consider either removing this export or updating minimal_init.lua to use it via vim.fn.getenv("PLENARY_DIR") for better configurability.

Suggested change
export PLENARY_DIR
# export PLENARY_DIR

Copilot uses AI. Check for mistakes.

# Change to the nvim config directory
cd "$NVIM_DIR"

# Run tests
echo -e "${YELLOW}Executing tests...${NC}"
nvim --headless \
-u tests/minimal_init.lua \
-c "lua require('plenary.test_harness').test_directory('tests/', { minimal_init = 'tests/minimal_init.lua', sequential = true })"

EXIT_CODE=$?

if [ $EXIT_CODE -eq 0 ]; then
echo -e "${GREEN}All tests passed!${NC}"
else
echo -e "${RED}Tests failed with exit code: $EXIT_CODE${NC}"
fi

exit $EXIT_CODE
109 changes: 109 additions & 0 deletions home-manager/programs/neovim/tests/utils_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
-- Tests for lua/utils.lua
-- Run with: nvim --headless -u tests/minimal_init.lua -c "PlenaryBustedDirectory tests/ { minimal_init = './tests/minimal_init.lua' }"

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment suggests running tests with PlenaryBustedDirectory, but the actual test runner script uses plenary.test_harness.test_directory(). Consider updating the comment to reflect the actual command used in run_tests.sh or removing it to avoid confusion.

Suggested change
-- Run with: nvim --headless -u tests/minimal_init.lua -c "PlenaryBustedDirectory tests/ { minimal_init = './tests/minimal_init.lua' }"
-- Run with: nvim --headless -u tests/minimal_init.lua -c "lua require('plenary.test_harness').test_directory('tests', {minimal_init = './tests/minimal_init.lua'})"

Copilot uses AI. Check for mistakes.

describe("utils", function()
local utils

before_each(function()
-- Clear any cached module to get fresh state
package.loaded["utils"] = nil
utils = require("utils")
end)

describe("cycle_buffer", function()
it("should be a function", function()
assert.is_function(utils.cycle_buffer)
end)

it("should not error with no buffers", function()
-- With only one buffer (current), should not error
assert.has_no.errors(function()
utils.cycle_buffer("next")
end)
end)

it("should not error when cycling previous", function()
assert.has_no.errors(function()
utils.cycle_buffer("prev")
end)
end)

it("should handle multiple buffers", function()
-- Create a second buffer
local buf1 = vim.api.nvim_get_current_buf()

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable buf1 is assigned but never used. Consider either using it for assertions or removing it to keep the code clean.

Suggested change
local buf1 = vim.api.nvim_get_current_buf()

Copilot uses AI. Check for mistakes.
vim.cmd("enew")
local buf2 = vim.api.nvim_get_current_buf()

-- Should be able to cycle between them
assert.has_no.errors(function()
utils.cycle_buffer("next")
end)

-- Clean up
vim.api.nvim_buf_delete(buf2, { force = true })
end)
Comment on lines +31 to +44

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test creates two buffers but doesn't verify that cycle_buffer actually switches between them. Consider adding assertions to check that the current buffer changes after calling cycle_buffer("next"), for example by comparing vim.api.nvim_get_current_buf() before and after the call.

Copilot uses AI. Check for mistakes.
end)
Comment on lines +13 to +45

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test for multiple buffers only checks for errors and doesn't verify that the buffer actually changed. Additionally, the cleanup logic will be skipped if an assertion fails. It's better to use after_each for robust cleanup and add assertions to confirm the buffer cycling behavior.

	describe("cycle_buffer", function()
		local bufs_to_clean = {}
		after_each(function()
			for _, b in ipairs(bufs_to_clean) do
				if vim.api.nvim_buf_is_valid(b) then
					vim.api.nvim_buf_delete(b, { force = true })
				end
			end
			bufs_to_clean = {}
		end)

		it("should be a function", function()
			assert.is_function(utils.cycle_buffer)
		end)

		it("should not error with only one buffer", function()
			assert.has_no.errors(function()
				utils.cycle_buffer("next")
			end)
			assert.has_no.errors(function()
				utils.cycle_buffer("prev")
			end)
		end)

		it("should cycle between multiple buffers", function()
			local orig_buf = vim.api.nvim_get_current_buf()

			-- Create a new buffer
			vim.cmd("enew")
			local new_buf = vim.api.nvim_get_current_buf()
			table.insert(bufs_to_clean, new_buf)
			assert.are_not.equal(orig_buf, new_buf, "New buffer should be different")

			-- We are on new_buf. Cycling next should go to orig_buf.
			utils.cycle_buffer("next")
			assert.are.equal(orig_buf, vim.api.nvim_get_current_buf(), "Should cycle to original buffer")

			-- Cycling next again should go back to new_buf.
			utils.cycle_buffer("next")
			assert.are.equal(new_buf, vim.api.nvim_get_current_buf(), "Should cycle back to new buffer")

			-- Cycling prev should go back to orig_buf.
			utils.cycle_buffer("prev")
			assert.are.equal(orig_buf, vim.api.nvim_get_current_buf(), "Should cycle prev to original buffer")
		end)
	end)


describe("copen", function()
it("should be a function", function()
assert.is_function(utils.copen)
end)

it("should handle empty quickfix list", function()
-- Clear quickfix list first
vim.fn.setqflist({}, "r")

assert.has_no.errors(function()
utils.copen()
end)
end)

it("should open quickfix window when items exist", function()
-- Add items to quickfix list
vim.fn.setqflist({
{ filename = "test.lua", lnum = 1, text = "Test item 1" },
{ filename = "test.lua", lnum = 2, text = "Test item 2" },
}, "r")

utils.copen()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium

Tests that interact with Neovim window APIs lack timeout protection. A hung operation could cause the test suite to wait until the CI timeout (30 minutes).

Consider adding timeout protection for tests that open windows or manipulate buffers. While plenary handles some timeouts internally, explicit guards for long-running operations would improve test reliability:

local timeout_ms = 5000
local start = vim.loop.now()
utils.copen()
assert.is_true(vim.loop.now() - start < timeout_ms, "Test operation timed out")

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#395
File: home-manager/programs/neovim/tests/utils_spec.lua#L68
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Tests that interact with Neovim window APIs lack timeout protection. A hung operation could cause the test suite to wait until the CI timeout (30 minutes).

Consider adding timeout protection for tests that open windows or manipulate buffers. While plenary handles some timeouts internally, explicit guards for long-running operations would improve test reliability:

```lua
local timeout_ms = 5000
local start = vim.loop.now()
utils.copen()
assert.is_true(vim.loop.now() - start < timeout_ms, "Test operation timed out")

</details>


-- Check if quickfix window is open
local qf_open = false
for _, win in ipairs(vim.api.nvim_list_wins()) do
local buf = vim.api.nvim_win_get_buf(win)
if vim.bo[buf].buftype == "quickfix" then
qf_open = true
break
end
end

assert.is_true(qf_open)

-- Clean up
vim.cmd("cclose")
vim.fn.setqflist({}, "r")
end)
Comment on lines +61 to +85

Copilot AI Dec 6, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copen function checks if quickfix size is greater than 1, but the test creates a quickfix list with 2 items. This means the test correctly validates the copen behavior when size > 1. However, consider adding a test for the edge case where size == 1 to verify that cclose is called as expected by the implementation logic.

Copilot uses AI. Check for mistakes.
end)
Comment on lines +47 to +86

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The cleanup logic here (cclose, setqflist) will not run if an assertion fails. Using an after_each block ensures that the quickfix list is cleared and the window is closed after each test in this suite, making the tests more robust and independent. Using pcall for cclose prevents errors if the window is already closed.

	describe("copen", function()
		after_each(function()
			pcall(vim.cmd, "cclose")
			vim.fn.setqflist({}, "r")
		end)

		it("should be a function", function()
			assert.is_function(utils.copen)
		end)

		it("should handle empty quickfix list", function()
			-- Clear quickfix list first
			vim.fn.setqflist({}, "r")

			assert.has_no.errors(function()
				utils.copen()
			end)
		end)

		it("should open quickfix window when items exist", function()
			-- Add items to quickfix list
			vim.fn.setqflist({
				{ filename = "test.lua", lnum = 1, text = "Test item 1" },
				{ filename = "test.lua", lnum = 2, text = "Test item 2" },
			}, "r")

			utils.copen()

			-- Check if quickfix window is open
			local qf_open = false
			for _, win in ipairs(vim.api.nvim_list_wins()) do
				local buf = vim.api.nvim_win_get_buf(win)
				if vim.bo[buf].buftype == "quickfix" then
					qf_open = true
					break
				end
			end

			assert.is_true(qf_open)
		end)
	end)


describe("cclear", function()
it("should be a function", function()
assert.is_function(utils.cclear)
end)

it("should clear the quickfix list", function()
-- Add items to quickfix list
vim.fn.setqflist({
{ filename = "test.lua", lnum = 1, text = "Test item" },
}, "r")

-- Verify it's not empty
assert.is_true(vim.fn.getqflist({ size = 0 }).size > 0)

-- Clear it
utils.cclear()

-- Verify it's empty
assert.equals(0, vim.fn.getqflist({ size = 0 }).size)
end)
end)
end)
Loading