[BUG] in neotest.lib.treesitter.parse_positions caused by neotest.lib.files.read

[ninofaivre]

NeoVim Version

NVIM v0.9.5
Build type: Release
LuaJIT 2.1.1702233742

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/share/nvim"

Describe the bug
I tried to make my own adapter so I looked over other adapter code and I ran into a problem using

require("neotest.lib").treesitter.parse_positions(valid_file_path, valid_query)

parse_position uses neotest.lib.files.read which itself uses nio.uv.fs_open : (as U can see I added a vim.print to log)

function neotest.lib.files.read(file_path)
  logger.debug("Reading file: " .. file_path)
  local open_err, file_fd = nio.uv.fs_open(file_path, "r", 438)
  vim.print("open err : " .. (open_err or "nil") .. "\nfile_fd : " .. (file_fd or "nil") .. "\n")
  assert(not open_err, open_err)
  -- ..............................
  -- ..............................

The problem here is that open_err and file_fd are mixed up
when file_path is a valid file_path with valid perm, etc... then open_err is just the fd and
the fd is nil because there is no error. When I intentionnaly feed it a wrong file_path, then
open_err is nil because there is no fd because the file_path is wrong and then file_fd
contains the error, causing the rest of the function to crash.

To Reproduce

• Go with the recommanded minimal.lua (did not changed anything to it)
• Do the following bash commands in your terminal :

cd /tmp
touch valid_file.txt
chmod 777 ./valid_file.txt

• nvim --clean -u minimal.lua (It's the default one)
  
• :lua require("neotest.lib").treesitter.parse_positions("/tmp/valid_file.txt", [[]], {})
  The assert should fail with the fd as an error.
  
• :lua require("neotest.lib").treesitter.parse_positions("/tmp/invalid_file_pathjldsqjldkfqklsjdfkljqklsdjfkljqklsdjfk", [[]], {})
  Here no assert should fail as the fd is nil (open_err in the implementation of neotest.lib.files.read) and then the function should
  crash because file_fd in the implementation now contains the error (a string I guess ? or at least it's printable).

Expected behavior
Don't really know what more to say here, to not mixup open_err and file_fd ?

Logs
As it's just a function that I'm trying to use to make an adapter and that fails I don't think there is any log to put here, instead I will reproduce every steps of the To Reproduce with the added vim.print and show you what I got.

valid file path :

invalid file path :

PS :

nvim nvim-config/pack/packer/start/neotest/lua/neotest/lib/file/init.lua

to add the vim.print at line 24 between fs_open and assert

[ninofaivre]

After digging a bit more and trying to patch this function myself by swapping error and res for all uv calls, I still have a problem with a wrong assert for the close. I thinked a bit about it and :

• 1. It's to much error and it would break almost any adapter so I must be one of the few to experience this error
• 2. You would not do that much error unintentionally

So my best guess is a wrong version of libluv / libuv with breaking changes between versions.
I don't know if you will be able to help me if the problem is that. I am running arch and installed neovim via pacman :
pacman -S neovim

[ninofaivre]

I think I start to understand my mistake thank's to this issue

[HiPhish]

How did you solve the problem? I am going through the same trouble writing a an adapter and I wanted to write tests for my discover_positions function. Something like this:

it('Discovers nothing in an empty file', function()
	vim.fn.writefile({''}, tempfile, 's')
	local tree = adapter.discover_positions(tempfile)
	assert.is_empty(tree)  -- Or something along those lines
end)

However, this won't work. Apparently fs_open needs to run inside an async context. Let's try again.

it('Discovers nothing in an empty file', function()
	vim.fn.writefile({''}, tempfile, 's')
	local task = nio.run(
		function()
			return adapter.discover_positions(tempfile)
		end,
		function(success, tree)
			assert.is_true(success)
			assert.is_true(false)
		end
	)
	task.wait()
end)

Well, this won't work either, I get a segfault and the test passes enough though the second assertions should definitely fail. It looks reasonable, but I am probably missing something here.

[rcarriga]

You can use the busted wrapper functions to run async tests.

Example:

neotest/tests/unit/lib/files/init_spec.lua

Lines 133 to 138 in f6048f3

	a.it("reads data", function()
	file:write("some data")
	file:flush()
	local read_data = files.read(path)
	assert.equal("some data", read_data)
	end)

[HiPhish]

@rcarriga I get the error that it is a nil.

Error → ./test/unit/discover_positions_spec.lua @ 4
Discovery of test positions
...e/nvim/site/pack/testing/start/neotest/lua/nio/tests.lua:35: attempt to call global 'it' (a nil value)

I have both Neotest and Plenary installed. Do I need some initial configuration first? I use the actual Busted to run tests, not Plenary's re-implementation, and all tests run in an environment separate from my own configuration and plugins.

EDIT:

The test file is:

local nio = require 'nio'
local adapter = require 'neotest-busted'

describe('Discovery of test positions', function()
	local tempfile

	before_each(function()
		-- Create temporary file
		tempfile = vim.fn.tempname()
	end)

	after_each(function()
		-- Delete temporary file
		if vim.fn.filereadable(tempfile) ~= 0 then
			vim.fn.delete(tempfile)
		end
	end)

	nio.tests.it('Discovers nothing in an empty file', function()
		vim.fn.writefile({''}, tempfile, 's')
		local result = adapter.discover_positions(tempfile)
		print(vim.inspect(result))
	end)
end)

[rcarriga]

Oh interesting, is the it function only defined within the scope of the test file then? The implementation is relatively so you could try copy it in the test file and see if it works. I don't use busted (TIL about the new adapter 😅) so have no easy way to test myself. If that works then that's a good start and we can try find a nice way to run them without having to manually wrap in each test file

[HiPhish]

There is no busted adapter yet, that's what I'm trying to write. For now I am running busted manually. I want to use tests to test the adapter implementation every step along the way, and I wrote a blog post about how to use busted to test Neovim plugins.

Oh interesting, is the it function only defined within the scope of the test file then?

Yeah, looks like some runtime magic. Within the test it is defined and I can call it indirectly (e.g. inside a loop).

The implementation is relatively so you could try copy it in the test file and see if it works.

Tried that already:

	it('Does something async', function()
		local success, err
		local task = nio.tasks.run(function()
				assert.is_true(true)
				assert.equal('x', 'y')
				return 'Yay!'
			end,
			function(success_, err_)
				success = success_
				if not success_ then
					err = err_
				end
			end)
		vim.wait(2000, function()
			return success ~= nil
		end, 20, false)

		if success == nil then
			error(string.format("Test task timed out\n%s", task.trace()))
		elseif not success then
			error(string.format("Test task failed with message:\n%s", err))
		end
	end)

This gives me a different error which is triggered by the failed assertion:

Discovery of test positions Does something async
vim/shared.lua:610: prefix: expected string, got table

If instead of a failing assertion I raise an error (e.g. error 'lol'), then I get an expected test failure:

Error → ./test/unit/discover_positions_spec.lua @ 19
Discovery of test positions Does something async
./test/unit/discover_positions_spec.lua:41: Test task failed with message:
The coroutine failed with this message: 
./test/unit/discover_positions_spec.lua:25: lol
stack traceback:
        [C]: in function 'error'
        ./test/unit/discover_positions_spec.lua:25: in function <./test/unit/discover_positions_spec.lua:21>

Should we open a new issue instead of spamming this one?