nvim-neotest / neotest

An extensible framework for interacting with tests within NeoVim.
MIT License
2.3k stars 115 forks source link

nio and busted? #366

Open HiPhish opened 7 months ago

HiPhish commented 7 months ago

Continuing the discussion from #349. I am currently writing an adapter for busted and I want to write tests for it in busted as well. I can run tests in general without issue following this blog post. The issue is when executing asynchronous functions like neotest.lib.treesitter.parse_positions.

Here is a simple test file:

local nio = require 'nio'

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)

    it('Always succeeds', function()
        assert.is_true(vim.endswith('abc', 'c'))
    end)
end)

As you can see this busted test is able to call into Neovim's vim module. Other than that it is a regular busted test. Now let's adding an actial test.

A naive attempt

    it('Discovers nothing in an empty file', function()
        local result = adapter.discover_positions(tempfile)
        print(vim.inspect(result))
    end)

This fails because discover_positions calls lib.treesitter.parse_positions without asynchronous context. The return values of lib.treesitter.parse_positions are wrong and the file descriptor will be returned as the error. Issue #349 describes the same behaviour.

Using nio.tests.it

    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)

This throws another error about it being nil inside to body of nio.tests.it.

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)

Replicate nio.tests.it

If the global it only exists within the test, how about we replicate the entire function inside the test?

    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 raises a validation error inside vim.startswith.

Error → ./test/unit/discover_positions_spec.lua @ 23
Discovery of test positions Does something async
vim/shared.lua:610: prefix: expected string, got table

This error is raised by the failing assertion. If the test function does not raise any errors then the test works fine.

HiPhish commented 7 months ago

Injecting it

Here is a hack that works: we explicitly inject it into nio.tests.it:

nio.tests.it = function(it, name, async_func)
  it(name, with_timeout(async_func, tonumber(vim.env.PLENARY_TEST_TIMEOUT)))
end

Now we can write the test:

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

It is kind of pointless to have separate it, before_each and after_each functions inside nio.tests if we have to pass the globals explicitly anyway. So we can add a metatable to nio.tests that lets us call the module and pass the global busted function:

local mt = {
    __call = function(_table, hook, name, async_func)
        hook(name, with_timeout(async_func, tonumber(vim.env.PLENARY_TEST_TIMEOUT)))
    end
}

setmetatable(nio.tests, mt)

With this the module remains backwards compatible and the test is less noisy:

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

What do you think? Is this an acceptable solution?

HiPhish commented 7 months ago

Dynamically resolve it and friends

A variant of the previous solution which preserves the existing API without making the module callable. Here we use the __index metamethod to automatically resolve it, before_each and after_each when they are requested.

local mt = {
    __index = function(_table, key)
        local env = getfenv(2)
        local hook = env[key]
        return function(name, async_func)
            hook(name, with_timeout(async_func, tonumber(vim.env.PLENARY_TEST_TIMEOUT)))
        end
    end
}

setmetatable(nio.tests, mt)

Now the test can be written as usual:

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