From d3693c1210bafb521fdc78ffc4417eb42df7e50c Mon Sep 17 00:00:00 2001 From: Krishna Balasubramanian Date: Mon, 2 Jun 2025 18:52:34 -0700 Subject: [PATCH 1/2] fix: handle NVIM environment variable in test script When Claude Code runs inside Neovim, the NVIM environment variable points to a socket path instead of the nvim executable. This caused the test script to fail with permission errors. The fix checks if NVIM is an executable file (not a socket) before using it, otherwise falls back to finding nvim in PATH. This allows tests to run properly both from within Claude Code/Neovim and from regular terminals. Added: - Test script to verify NVIM detection logic works correctly - Documentation in DEVELOPMENT.md explaining the behavior --- DEVELOPMENT.md | 14 +++++++++ scripts/test.sh | 17 ++++++---- scripts/test_nvim_detection.sh | 57 ++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) create mode 100755 scripts/test_nvim_detection.sh diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index bd9cb0f..2531885 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -183,6 +183,20 @@ make test-basic make test-config ``` +### Running Tests from Within Neovim/Claude Code + +When running tests from within a Neovim instance (such as when using Claude Code via claude-code.nvim), the test script automatically handles the `$NVIM` environment variable which normally points to a socket file instead of the nvim executable. + +The test script will: +- Use the `$NVIM` variable if it points to a valid executable file +- Fall back to finding `nvim` in `$PATH` if `$NVIM` points to a socket or invalid path +- Display which nvim binary is being used for transparency + +To verify the NVIM detection logic works correctly, you can run: +```bash +./scripts/test_nvim_detection.sh +``` + ### Writing Tests Tests are written in Lua using a simple BDD-style API: diff --git a/scripts/test.sh b/scripts/test.sh index 529dd6a..4a02fcc 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -12,12 +12,17 @@ cd "$PLUGIN_DIR" # Print current directory for debugging echo "Running tests from: $(pwd)" -# Find nvim -NVIM=${NVIM:-$(which nvim)} - -if [ -z "$NVIM" ]; then - echo "Error: nvim not found in PATH" - exit 1 +# Find nvim - ignore NVIM env var if it points to a socket +if [ -n "$NVIM" ] && [ -x "$NVIM" ] && [ ! -S "$NVIM" ]; then + # NVIM is set and is an executable file (not a socket) + echo "Using NVIM from environment: $NVIM" +else + # Find nvim in PATH + NVIM=$(which nvim) + if [ -z "$NVIM" ]; then + echo "Error: nvim not found in PATH" + exit 1 + fi fi echo "Running tests with $NVIM" diff --git a/scripts/test_nvim_detection.sh b/scripts/test_nvim_detection.sh new file mode 100755 index 0000000..1bc14c3 --- /dev/null +++ b/scripts/test_nvim_detection.sh @@ -0,0 +1,57 @@ +#!/bin/bash +set -e + +# Test script to verify NVIM environment variable detection logic +# This script tests the fix for handling NVIM variable when running inside Neovim + +echo "Testing NVIM environment variable detection..." + +# Save original NVIM value +ORIGINAL_NVIM="$NVIM" + +# Test 1: NVIM points to a socket (simulating inside Neovim) +echo "Test 1: NVIM points to a socket" +export NVIM="/tmp/test_socket" +mkfifo "$NVIM" 2>/dev/null || true # Create a named pipe (similar to socket) +if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then + echo "✓ Fallback to PATH works" +else + echo "✗ Fallback failed" +fi +rm -f "$NVIM" + +# Test 2: NVIM points to valid executable +echo "Test 2: NVIM points to valid executable" +export NVIM="$(which nvim)" +if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Using NVIM from environment"; then + echo "✓ Using provided NVIM works" +else + echo "✗ Using provided NVIM failed" +fi + +# Test 3: NVIM points to non-existent path +echo "Test 3: NVIM points to non-existent path" +export NVIM="/nonexistent/nvim" +if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then + echo "✓ Fallback from invalid path works" +else + echo "✗ Fallback from invalid path failed" +fi + +# Test 4: NVIM is unset +echo "Test 4: NVIM is unset" +unset NVIM +if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then + echo "✓ Unset NVIM works" +else + echo "✗ Unset NVIM failed" +fi + +# Restore original NVIM value +if [ -n "$ORIGINAL_NVIM" ]; then + export NVIM="$ORIGINAL_NVIM" +else + unset NVIM +fi + +echo "All NVIM detection tests completed!" \ No newline at end of file From da466772cb44e6360f663aca964f9e0707653631 Mon Sep 17 00:00:00 2001 From: Krishna Balasubramanian Date: Tue, 3 Jun 2025 16:21:36 -0700 Subject: [PATCH 2/2] Address CodeRabbit feedback: improve script robustness and formatting - Replace 'which' with POSIX-compliant 'command -v' in test scripts - Fix markdown formatting issues in DEVELOPMENT.md (add blank lines) - Separate variable declaration and assignment in test_nvim_detection.sh - Add robust absolute path resolution using readlink and BASH_SOURCE - Enhance error handling with 'set -euo pipefail' for strict mode - Replace unreliable $0 references in bash -c contexts These changes improve portability, reliability, and follow shell scripting best practices while addressing all linting and formatting issues. --- DEVELOPMENT.md | 2 ++ scripts/test.sh | 3 +-- scripts/test_nvim_detection.sh | 20 ++++++++++++++------ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 2531885..06bfef7 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -188,11 +188,13 @@ make test-config When running tests from within a Neovim instance (such as when using Claude Code via claude-code.nvim), the test script automatically handles the `$NVIM` environment variable which normally points to a socket file instead of the nvim executable. The test script will: + - Use the `$NVIM` variable if it points to a valid executable file - Fall back to finding `nvim` in `$PATH` if `$NVIM` points to a socket or invalid path - Display which nvim binary is being used for transparency To verify the NVIM detection logic works correctly, you can run: + ```bash ./scripts/test_nvim_detection.sh ``` diff --git a/scripts/test.sh b/scripts/test.sh index 4a02fcc..ab2b348 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -18,8 +18,7 @@ if [ -n "$NVIM" ] && [ -x "$NVIM" ] && [ ! -S "$NVIM" ]; then echo "Using NVIM from environment: $NVIM" else # Find nvim in PATH - NVIM=$(which nvim) - if [ -z "$NVIM" ]; then + if ! NVIM=$(command -v nvim); then echo "Error: nvim not found in PATH" exit 1 fi diff --git a/scripts/test_nvim_detection.sh b/scripts/test_nvim_detection.sh index 1bc14c3..5f52349 100755 --- a/scripts/test_nvim_detection.sh +++ b/scripts/test_nvim_detection.sh @@ -1,9 +1,13 @@ #!/bin/bash -set -e +set -euo pipefail # Test script to verify NVIM environment variable detection logic # This script tests the fix for handling NVIM variable when running inside Neovim +# Get the absolute directory path of this script +SCRIPT_DIR="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" +PROJECT_DIR="$(dirname "$SCRIPT_DIR")" + echo "Testing NVIM environment variable detection..." # Save original NVIM value @@ -13,7 +17,7 @@ ORIGINAL_NVIM="$NVIM" echo "Test 1: NVIM points to a socket" export NVIM="/tmp/test_socket" mkfifo "$NVIM" 2>/dev/null || true # Create a named pipe (similar to socket) -if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then +if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then echo "✓ Fallback to PATH works" else echo "✗ Fallback failed" @@ -22,8 +26,12 @@ rm -f "$NVIM" # Test 2: NVIM points to valid executable echo "Test 2: NVIM points to valid executable" -export NVIM="$(which nvim)" -if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Using NVIM from environment"; then +if ! NVIM=$(command -v nvim); then + echo "Error: nvim not found in PATH" + exit 1 +fi +export NVIM +if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Using NVIM from environment"; then echo "✓ Using provided NVIM works" else echo "✗ Using provided NVIM failed" @@ -32,7 +40,7 @@ fi # Test 3: NVIM points to non-existent path echo "Test 3: NVIM points to non-existent path" export NVIM="/nonexistent/nvim" -if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then +if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then echo "✓ Fallback from invalid path works" else echo "✗ Fallback from invalid path failed" @@ -41,7 +49,7 @@ fi # Test 4: NVIM is unset echo "Test 4: NVIM is unset" unset NVIM -if timeout 5 bash -c 'cd "$(dirname "$0")" && ./scripts/test.sh' 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then +if timeout 5 bash -c "cd '$PROJECT_DIR' && ./scripts/test.sh" 2>&1 | head -10 | grep -q "Running tests with.*nvim"; then echo "✓ Unset NVIM works" else echo "✗ Unset NVIM failed"